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.

Sign up for GitHub

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

Jump to bottom

[close #375] warm up RawKVClient while creating it #367

Merged
merged 10 commits into from
Dec 9, 2021
Merged

[close #375] warm up RawKVClient while creating it #367

merged 10 commits into from
Dec 9, 2021

Conversation

iosmanthus
Copy link
Member

@iosmanthus iosmanthus commented Dec 8, 2021 •
edited

Signed-off-by: iosmanthus myosmanthustree@gmail.com

What problem does this PR solve?

We find some initialization work that needs to do in the first gRPC call in RawKVClient which might result in a timeout in the first few gRPC calls.

First run:

image

Second run:

image

What is changed and how it works?

To solve this problem, we may launch a raw_get request to warm up the runtime.

Check List

Tests

  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@iosmanthus
warm up RawKVClient while creating it
80c67ca 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@birdstorm
Copy link
Collaborator

IMHO not suitable to warm up the client by calling get() when initializing.

@zz-jason zz-jason changed the title warm up RawKVClient while creating it [close #375] warm up RawKVClient while creating it Dec 9, 2021
@birdstorm
try fix by load class
0100efd 
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
@birdstorm
add load class from jar file
c40b61c 
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
@birdstorm
warm up solution 2
e6dd4ef 
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
@birdstorm
format
74c9339 
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
@birdstorm
resolve comments
a2ac28c 
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
@birdstorm
add logs
c012659 
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
@birdstorm
remove test
2ed1fe8 
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
zz-jason
zz-jason approved these changes Dec 9, 2021
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

marsishandsome
marsishandsome approved these changes Dec 9, 2021
View reviewed changes
Copy link
Collaborator

@marsishandsome marsishandsome left a comment

Choose a reason for hiding this comment

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

LGTM

@marsishandsome
Copy link
Collaborator

/merge

@ti-srebot
Copy link
Collaborator

/run-all-tests

@zz-jason zz-jason merged commit c95479e into tikv:master Dec 9, 2021
ti-srebot pushed a commit to ti-srebot/client-java that referenced this pull request Dec 9, 2021
@iosmanthus @ti-srebot
cherry pick tikv#367 to release-3.1
e419210 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Dec 9, 2021
Merged
@ti-srebot
Copy link
Collaborator

cherry pick to release-3.1 in PR #381

zz-jason pushed a commit that referenced this pull request Dec 9, 2021
@ti-srebot
[close #375] warm up RawKVClient while creating it (#367) (#381)
18c655b
ankita25 pushed a commit to ankita25/client-java that referenced this pull request Dec 14, 2021
@iosmanthus @ankita25
[close tikv#375] warm up RawKVClient while creating it (tikv#367)
f6d7ff6 
Signed-off-by: Ankita Wagh <awagh@pinterest.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zz-jason zz-jason zz-jason approved these changes

@marsishandsome marsishandsome marsishandsome approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

@birdstorm birdstorm Awaiting requested review from birdstorm

Assignees
No one assigned
Labels
component/common component/rawkv status/can-merge status/LGT2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@iosmanthus @birdstorm @marsishandsome @ti-srebot @zz-jason