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

Cherry pick of #11330 on release-3.4 #11734

Conversation

jingyih
Copy link
Contributor

@jingyih jingyih commented Mar 30, 2020

Cherry pick of #11330 on release-3.4.

#11330: mvcc/kvstore:when the number key-value is greater than one

Modified ./test to automatically detect branch when finding merge base. (Thanks to @jpbetz)

…ompact take too long and blocks other requests
@jingyih jingyih changed the title Automated cherry pick of #11330 Automated cherry pick of #11330 on release-3.4 Mar 30, 2020
@jingyih
Copy link
Contributor Author

jingyih commented Mar 30, 2020

Technically this is a performance improvement, not a bug fix. Given that we do not have a plan to release etcd 3.5 soon, I would like to see how people feel about backporting this performance improvement to etcd 3.4 (so that everybody can benefit from it early).

Basically, when cluster is large, the compaction of the in-memory index tree could take a long time to finish (hundreds of milliseconds). While server applies the compaction log entry, other requests (including linearizable read requests) might be blocked waiting on it.

@jingyih
Copy link
Contributor Author

jingyih commented Mar 30, 2020

cc @gyuho @xiang90 @spzala @jpbetz @WIZARD-CXY

@gyuho
Copy link
Contributor

gyuho commented Mar 30, 2020

can fix CI? otherwise, lgtm

@jpbetz
Copy link
Contributor

jpbetz commented Mar 30, 2020

It's my fault that fmt is failing, but I think it can be fixed by switching from master to release-3.4 here: https://github.com/etcd-io/etcd/blob/master/test#L584. We should improve that test code automatically detect the correct branch, e.g.

git log --oneline "$(git merge-base HEAD $(git rev-parse --abbrev-ref --symbolic-full-name @{u}))"...HEAD

Would someone be willing to apply this to the release branches and master?

@jingyih You can apply to /test on this PR?

@WIZARD-CXY
Copy link
Contributor

lgtm

@jingyih
Copy link
Contributor Author

jingyih commented Mar 31, 2020

@jingyih You can apply to /test on this PR?

Done

@jingyih jingyih changed the title Automated cherry pick of #11330 on release-3.4 Cherry pick of #11330 on release-3.4 Mar 31, 2020
@jingyih
Copy link
Contributor Author

jingyih commented Mar 31, 2020

shellcheck is failing, which I cannot reproduce locally. Let me try to debug and fix it here.

test Outdated
@@ -581,7 +581,7 @@ function receiver_name_pass {
}

function commit_title_pass {
git log --oneline "$(git merge-base HEAD master)"...HEAD | while read -r l; do
git log --oneline "$(git merge-base HEAD $(git rev-parse --abbrev-ref --symbolic-full-name @{u}))"...HEAD | while read -r l; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Latest test failure:

test:584:43: warning: Quote this to prevent word splitting. [SC2046]
test:584:94: warning: This { is literal. Check expression (missing ;/
?) or quote it. [SC1083]
test:584:96: warning: This } is literal. Check expression (missing ;/
?) or quote it. [SC1083]

We might need to do something like:

gitBranch=$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}")
gitBaseHash=$(git merge-base HEAD "${gitBranch}")
git log --oneline "${gitBaseHash}"...HEAD | while read -r l; do

Fair warning: I haven't run this. It needs to be tested with shellcheck.

@wojtek-t
Copy link
Contributor

/cc

@jingyih jingyih force-pushed the automated-cherry-pick-of-#11330-upstream-release-3.4 branch from b1fe057 to 62f0248 Compare March 31, 2020 17:56
@jingyih jingyih force-pushed the automated-cherry-pick-of-#11330-upstream-release-3.4 branch from 62f0248 to 5f17aa2 Compare March 31, 2020 18:00
@gyuho gyuho merged commit 857dffa into etcd-io:release-3.4 Mar 31, 2020
@gyuho
Copy link
Contributor

gyuho commented Mar 31, 2020

I will cut a new release today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants