bindinfo: add timeout for loading binding from storage #51550

hawkingrei · 2024-03-06T10:59:25Z

What problem does this PR solve?

Issue Number: ref #51347

Problem Summary:

What changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

hawkingrei · 2024-03-06T11:53:18Z

/retest

codecov · 2024-03-06T11:58:06Z

Codecov Report

Merging #51550 (d8c7622) into master (15947c1) will increase coverage by 2.1180%.
Report is 4 commits behind head on master.
The diff coverage is 76.9230%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #51550        +/-   ##
================================================
+ Coverage   70.7427%   72.8608%   +2.1180%     
================================================
  Files          1473       1473                
  Lines        436986     437972       +986     
================================================
+ Hits         309136     319110      +9974     
+ Misses       108511      98900      -9611     
- Partials      19339      19962       +623

Flag	Coverage Δ
integration	`49.1051% <55.7692%> (?)`
unit	`70.5752% <76.9230%> (+0.0230%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`53.9957% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`51.4483% <ø> (+5.5786%)`	⬆️

hawkingrei · 2024-03-06T15:30:51Z

/retest

hawkingrei · 2024-03-07T11:21:13Z

/retest

hawkingrei · 2024-03-08T01:41:28Z

/retest

pkg/sessionctx/variable/sysvar.go

pkg/bindinfo/binding_cache.go

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

Co-authored-by: Yuanjia Zhang <qw4990@163.com>

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

AilinKid · 2024-03-12T09:36:24Z

pkg/bindinfo/binding_match.go

 		return binding, matched, metrics.ScopeGlobal
 	}
+


means return binding, matched, nil at last?

Yes, actually there have been no changes here.

pkg/bindinfo/binding_cache.go

AilinKid · 2024-03-12T09:44:15Z

tests/integrationtest/r/executor/merge_join.result

-    │   └─TableReader	6656.67	root		data:Selection
-    │     └─Selection	6656.67	cop[tikv]		ne(executor__merge_join.t1.c1, 1), not(isnull(executor__merge_join.t1.c1))
-    │       └─TableFullScan	10000.00	cop[tikv]	table:t1	keep order:false, stats:pseudo
+    │ └─TableReader	6656.67	root		data:Selection


why this is changed?

it has be reverted.

AilinKid · 2024-03-12T09:44:35Z

tests/integrationtest/r/executor/partition/partition_boundaries.result

-TableReader	0.00	root	partition:dual	data:Selection
-└─Selection	0.00	cop[tikv]		eq(executor__partition__partition_boundaries.t.a, 3000000)
-  └─TableFullScan	14.00	cop[tikv]	table:t	keep order:false
+TableDual	0.00	root		rows:0


same as above

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

ti-chi-bot · 2024-03-12T10:04:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AilinKid, easonn7, qw4990

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [AilinKid,qw4990]
~~pkg/sessionctx/variable/OWNERS~~ [easonn7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-03-12T10:04:45Z

[LGTM Timeline notifier]

Timeline:

2024-03-12 09:31:05.757466326 +0000 UTC m=+764292.779712714: ☑️ agreed by qw4990.
2024-03-12 10:04:44.200743182 +0000 UTC m=+766311.222989571: ☑️ agreed by AilinKid.

hawkingrei · 2024-03-12T11:55:39Z

/reteat

hawkingrei · 2024-03-12T11:57:47Z

/retest

hawkingrei · 2024-03-12T12:36:58Z

/retest

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed do-not-merge/needs-linked-issue labels Mar 6, 2024

hawkingrei mentioned this pull request Mar 6, 2024

planner: Plan Binding Enhancement #51347

Open

18 tasks

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label Mar 6, 2024

hawkingrei changed the title ~~bindinfo: add timeout for loading binding from storage~~ bindinfo: add timeout for loading binding from storage [WIP] Mar 6, 2024

hawkingrei force-pushed the add_timeout_for_load_binding branch from e04e9b8 to c5aa2db Compare March 7, 2024 07:57

hawkingrei force-pushed the add_timeout_for_load_binding branch from 2f65e65 to 685f1f9 Compare March 7, 2024 09:08

hawkingrei changed the title ~~bindinfo: add timeout for loading binding from storage [WIP]~~ bindinfo: add timeout for loading binding from storage Mar 7, 2024

hawkingrei force-pushed the add_timeout_for_load_binding branch from ebb3d9e to b5f60c3 Compare March 8, 2024 00:21

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 8, 2024

hawkingrei force-pushed the add_timeout_for_load_binding branch from b25ac50 to c57f8d3 Compare March 10, 2024 09:19

hawkingrei requested a review from qw4990 March 11, 2024 07:21

qw4990 reviewed Mar 11, 2024

View reviewed changes

pkg/sessionctx/variable/sysvar.go Outdated Show resolved Hide resolved

qw4990 reviewed Mar 11, 2024

View reviewed changes

pkg/sessionctx/variable/sysvar.go Outdated Show resolved Hide resolved

qw4990 reviewed Mar 11, 2024

View reviewed changes

pkg/bindinfo/binding_cache.go Outdated Show resolved Hide resolved

hawkingrei force-pushed the add_timeout_for_load_binding branch 2 times, most recently from e7b7b99 to b898638 Compare March 11, 2024 10:43

hawkingrei and others added 10 commits March 12, 2024 17:28

update

b2c34c3

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

update

e43c3ba

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

Update pkg/bindinfo/binding_cache.go

2e3a501

Co-authored-by: Yuanjia Zhang <qw4990@163.com>

update

d588b08

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

update

b8da5da

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

update

eb86c86

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

update

c6ca703

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

update

8d57fbe

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

update

85fa14f

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

update

dbf2229

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

hawkingrei force-pushed the add_timeout_for_load_binding branch from 5ad337e to dbf2229 Compare March 12, 2024 09:29

update

da14732

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

qw4990 approved these changes Mar 12, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 12, 2024

linter

5ab713d

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

AilinKid reviewed Mar 12, 2024

View reviewed changes

hawkingrei added 3 commits March 12, 2024 17:53

linter

d390722

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

linter

bcb5de1

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

linter

d8c7622

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

AilinKid approved these changes Mar 12, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 12, 2024

ti-chi-bot bot merged commit 46e95f4 into pingcap:master Mar 12, 2024
22 of 23 checks passed

hawkingrei deleted the add_timeout_for_load_binding branch March 13, 2024 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bindinfo: add timeout for loading binding from storage #51550

bindinfo: add timeout for loading binding from storage #51550

hawkingrei commented Mar 6, 2024 •

edited

Loading

hawkingrei commented Mar 6, 2024

codecov bot commented Mar 6, 2024 •

edited

Loading

hawkingrei commented Mar 6, 2024

hawkingrei commented Mar 7, 2024

hawkingrei commented Mar 8, 2024

AilinKid Mar 12, 2024

hawkingrei Mar 12, 2024

AilinKid Mar 12, 2024

hawkingrei Mar 12, 2024

AilinKid Mar 12, 2024

hawkingrei Mar 12, 2024

ti-chi-bot bot commented Mar 12, 2024

ti-chi-bot bot commented Mar 12, 2024

hawkingrei commented Mar 12, 2024

hawkingrei commented Mar 12, 2024

hawkingrei commented Mar 12, 2024

bindinfo: add timeout for loading binding from storage #51550

bindinfo: add timeout for loading binding from storage #51550

Conversation

hawkingrei commented Mar 6, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

hawkingrei commented Mar 6, 2024

codecov bot commented Mar 6, 2024 • edited Loading

Codecov Report

hawkingrei commented Mar 6, 2024

hawkingrei commented Mar 7, 2024

hawkingrei commented Mar 8, 2024

AilinKid Mar 12, 2024

Choose a reason for hiding this comment

hawkingrei Mar 12, 2024

Choose a reason for hiding this comment

AilinKid Mar 12, 2024

Choose a reason for hiding this comment

hawkingrei Mar 12, 2024

Choose a reason for hiding this comment

AilinKid Mar 12, 2024

Choose a reason for hiding this comment

hawkingrei Mar 12, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Mar 12, 2024

ti-chi-bot bot commented Mar 12, 2024

[LGTM Timeline notifier]

hawkingrei commented Mar 12, 2024

hawkingrei commented Mar 12, 2024

hawkingrei commented Mar 12, 2024

hawkingrei commented Mar 6, 2024 •

edited

Loading

codecov bot commented Mar 6, 2024 •

edited

Loading