Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add explicit request source type to label the external request like lightning/br #868

Merged
merged 8 commits into from
Jul 7, 2023

Conversation

nolouch
Copy link
Contributor

@nolouch nolouch commented Jul 4, 2023

if use

set @tidb_request_source_type='lightling'
select count(*) from t;

the request source field will like:

external_Select_lightning

util/request_source.go Outdated Show resolved Hide resolved
util/request_source.go Outdated Show resolved Hide resolved
@nolouch nolouch force-pushed the explicit-request-source branch from 16dabbc to e7c5520 Compare July 5, 2023 07:42
@nolouch nolouch force-pushed the explicit-request-source branch 2 times, most recently from 7e79174 to 157d072 Compare July 5, 2023 07:51
util/request_source.go Outdated Show resolved Hide resolved
Signed-off-by: nolouch <[email protected]>
@nolouch nolouch force-pushed the explicit-request-source branch 2 times, most recently from 49f8e28 to 0f935d8 Compare July 5, 2023 09:52
Signed-off-by: nolouch <[email protected]>
@nolouch nolouch force-pushed the explicit-request-source branch from 0f935d8 to b2b79e4 Compare July 5, 2023 09:54
util/request_source.go Outdated Show resolved Hide resolved
@nolouch nolouch force-pushed the explicit-request-source branch from 245b9e2 to 69d736f Compare July 6, 2023 02:55
util/request_source_test.go Outdated Show resolved Hide resolved
@nolouch nolouch force-pushed the explicit-request-source branch from 69d736f to b2b79e4 Compare July 6, 2023 03:11
@nolouch
Copy link
Contributor Author

nolouch commented Jul 6, 2023

ptal @HuSharp @glorv

}
return ExternalRequest + r.RequestSourceType
labels := []string{ExternalRequest, r.RequestSourceType, r.ExplicitRequestSourceType}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, tikv uses the last segments of resquest_source.split("_") as the explicit_request_source(task_names) value, so if it is empty, the logic won't be correct. Because there are some request_source_types such as ddl_refill, we can't depend on the number of segments either. So I suggest always make ExplicitRequestSourceType not empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed,ptal @glorv

Signed-off-by: nolouch <[email protected]>
Copy link
Contributor

@glorv glorv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

}
explicitSourceType := r.ExplicitRequestSourceType
if len(explicitSourceType) == 0 {
explicitSourceType = ExplicitTypeDB
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: not sure if db is a good name for the default value. Maybe default is more conventional?

Copy link
Member

@HuSharp HuSharp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Signed-off-by: nolouch <[email protected]>
@nolouch nolouch force-pushed the explicit-request-source branch from c3bf88b to bedfada Compare July 6, 2023 08:56
util/request_source.go Outdated Show resolved Hide resolved
util/request_source.go Outdated Show resolved Hide resolved
Signed-off-by: nolouch <[email protected]>
@nolouch nolouch force-pushed the explicit-request-source branch 2 times, most recently from 558a4af to 144302b Compare July 7, 2023 05:49
@overvenus
Copy link
Member

overvenus commented Jul 7, 2023

Some tests fail.

@nolouch nolouch force-pushed the explicit-request-source branch from 144302b to 3d194d0 Compare July 7, 2023 06:34
@HuSharp
Copy link
Member

HuSharp commented Jul 7, 2023

@overvenus Can you leave a merge label? thx!

@overvenus
Copy link
Member

/merge

@overvenus overvenus merged commit 178f6fa into tikv:master Jul 7, 2023
@nolouch nolouch deleted the explicit-request-source branch July 7, 2023 07:37
nolouch added a commit to nolouch/client-go that referenced this pull request Jul 14, 2023
iosmanthus added a commit that referenced this pull request Aug 11, 2023
* client-go: add some key range info to error when PD returned no region (#862)

Signed-off-by: Chao Wang <[email protected]>

* *: refine non-global stale-read request retry logic (#863)

Signed-off-by: crazycs520 <[email protected]>

* Fix the issue that primary pessimistic lock may be left not cleared after GC (#866)

* Fix the issue that primary pessimistic lock may be left not cleared after GC

Signed-off-by: MyonKeminta <[email protected]>

* Fix mysteriously shown up thing that makes compilation failed

Signed-off-by: MyonKeminta <[email protected]>

* Fix test effectiveness (forgot to set txn2 to pessimistic txn); add more strict checks

Signed-off-by: MyonKeminta <[email protected]>

* Address comments

Signed-off-by: MyonKeminta <[email protected]>

---------

Signed-off-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>

* add explicit request source type to label the external request like lightning/br (#868)

Signed-off-by: nolouch <[email protected]>

* use '%d' instead of '%q' for some int values in error message (#875)

Signed-off-by: Chao Wang <[email protected]>

* format key in error message in method `scanRegions` (#876)

Signed-off-by: Chao Wang <[email protected]>

* make cop request timeout a config paramter (#865)

* update

Signed-off-by: Spade A <[email protected]>

* update

Signed-off-by: Spade A <[email protected]>

* update

Signed-off-by: Spade A <[email protected]>

* update

Signed-off-by: Spade A <[email protected]>

---------

Signed-off-by: Spade A <[email protected]>

* region_cache: support check pending tiflash peer (#821)

Signed-off-by: guo-shaoge <[email protected]>
Co-authored-by: disksing <[email protected]>

* *: add `SnapshotIterReverse` and make `iterReverse` supports `lowerBound` (#883)

Signed-off-by: Jason Mo <[email protected]>

* *: fix stale read ops metric (#878) (#889)

Signed-off-by: crazycs520 <[email protected]>
Co-authored-by: disksing <[email protected]>

* add gc options (#828)

Signed-off-by: weedge <[email protected]>
Co-authored-by: disksing <[email protected]>

* reload region cache when store is resolved from invalid status (#843)

Signed-off-by: you06 <[email protected]>
Co-authored-by: disksing <[email protected]>

* ci: update setup-go action (#904)

Signed-off-by: disksing <[email protected]>

* fix unexpected slow query during GC running after stop 1 tikv-server (#899) (#909)

* fix unexpected slow query during GC running after stop 1 tikv-server

Signed-off-by: crazycs520 <[email protected]>

* fix test

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>

* resource_manager: ignore ru metrics for background request (#872)

Signed-off-by: husharp <[email protected]>
Co-authored-by: disksing <[email protected]>

* add more log for diagnose (#915)

* add more log for diagnose

Signed-off-by: crazycs520 <[email protected]>

* fix

Signed-off-by: crazycs520 <[email protected]>

* add more log for diagnose

Signed-off-by: crazycs520 <[email protected]>

* add more log

Signed-off-by: crazycs520 <[email protected]>

* address comment

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>

* use context logger as much as possible (#908)

* use context logger as much as possible

Signed-off-by: crazycs520 <[email protected]>

* refine

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>

* Resume max retry time check for stale read retry with leader option(#903) (#911)

* Resume max retry time check for stale read retry with leader option

Signed-off-by: cfzjywxk <[email protected]>

* add cancel

Signed-off-by: cfzjywxk <[email protected]>

---------

Signed-off-by: cfzjywxk <[email protected]>

* request_source: remove default label (#890)

* request_source: remove default label

Signed-off-by: nolouch <[email protected]>

* add a function to set request source task type (#925)

* add a function to set request source task type

Signed-off-by: glorv <[email protected]>

* ci: update go version (#936)

* ci: update go version

Signed-off-by: crazycs520 <[email protected]>

* fix test

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>

* use tidb_kv_read_timeout as first kv request timeout (#919)

* support tidb_kv_read_timeout as first round kv request timeout

Signed-off-by: crazycs520 <[email protected]>

* fix ci

Signed-off-by: crazycs520 <[email protected]>

* fix ci

Signed-off-by: crazycs520 <[email protected]>

* fix ci

Signed-off-by: crazycs520 <[email protected]>

* fix ci

Signed-off-by: crazycs520 <[email protected]>

* fix ci

Signed-off-by: crazycs520 <[email protected]>

* update comment

Signed-off-by: crazycs520 <[email protected]>

* refine test

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>

* [pick] resource_control: bypass some internal urgent request (#938)

* resource_control: bypass some internal urgent request (#884)

Signed-off-by: nolouch <[email protected]>

* resourcecontrol: fix nil pointer (#900)

Signed-off-by: nolouch <[email protected]>

---------

Signed-off-by: nolouch <[email protected]>

---------

Signed-off-by: Chao Wang <[email protected]>
Signed-off-by: crazycs520 <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: nolouch <[email protected]>
Signed-off-by: Spade A <[email protected]>
Signed-off-by: guo-shaoge <[email protected]>
Signed-off-by: Jason Mo <[email protected]>
Signed-off-by: weedge <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: disksing <[email protected]>
Signed-off-by: husharp <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: glorv <[email protected]>
Signed-off-by: iosmanthus <[email protected]>
Co-authored-by: 王超 <[email protected]>
Co-authored-by: crazycs <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
Co-authored-by: ShuNing <[email protected]>
Co-authored-by: Spade  A <[email protected]>
Co-authored-by: guo-shaoge <[email protected]>
Co-authored-by: disksing <[email protected]>
Co-authored-by: Hangjie Mo <[email protected]>
Co-authored-by: weedge <[email protected]>
Co-authored-by: you06 <[email protected]>
Co-authored-by: Hu# <[email protected]>
Co-authored-by: cfzjywxk <[email protected]>
Co-authored-by: glorv <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants