ZOOKEEPER-3589: 3.4-branch has potential data inconsistency caused by… #1123
Conversation
|
Looking |
|
The original patch is #583 from @lvfangmin |
There was a problem hiding this comment.
Thanks for doing this.
I am trying to 'diff' this diff with the original one but the original one is full of reformat actions. btw I found some little difference, I left a comment.
It would be also good to state in the commit message that this patch is a backport of the work of @lvfangmin and cite him as original author.
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
Show resolved
Hide resolved
| import org.junit.After; | ||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
| import org.junit.Ignore; |
There was a problem hiding this comment.
Can you please fix the order of imports?
In branch 3.4 we do not have checkstyle
There was a problem hiding this comment.
OK. I will run checkstyle for this commit.
|
@yfxhust While I support the efforts of backporting critical bugfixes to 3.4, have you considered how much hassle would it be to upgrade your project to 3.5? We highly encourage projects to upgrade sooner rather than later, because we'd like to EOL 3.4 soon. Also the community is focusing on 3.6.0 release and even if this patch gets submitted, I cannot tell you when are we going to have a new release of 3.4 branch. |
|
@eolivelli Sorry for my miss for citing original author @lvfangmin. I will add it in my commit. |
@anmolnar The situation is not easy in real micro services environment. Zookeeper is a very foundamental component and many service depends on it. Most of these services use apache curator as client. We can not upgrade server cluster before the curator version is upgraded. In a real product micro service environment, curator client upgrading is not easy and it depends on many factors that are not from technique view. Curator's zookeeper compatibility need more consideration. In my opinion, any known critical bugs should be fixed before EOL. |
… ZOOKEEPER-3104 This patch is a backport for the work of ZOOKEEPER-3104. I adapt the original patch to branch-3.4. Original Author: Allan Lyu <fangmin@apache.org> Author: Fangxi Yin <yinfangxi@kuaishou.com>
…ports for QuorumPeerMainTest. Make code comments of forceSnapSync more accurate.
yfxhust
force-pushed
the
ZOOKEEPER-3589
branch
from
137bf44 to
7fe17fe
Compare
I agree. I just wanted to tell you the reality. |
lvfangmin
left a comment
lvfangmin
left a comment
There was a problem hiding this comment.
Thanks @yfxhust for porting this back to 3.4.
+1
The patch itself looks good to me, the main change is moving the NEWLEADER packet part to the time when we finished sending snapshot to the socket. The forceSnapSync option was added mainly for the testing purpose.
I would suggest to put some effort on the 3.4 to 3.6 migration, I understand the migration is painful and risky, but it was much more smooth than what we thought will be when we did that inside FB, and you can benefit from the latest cool feature and bug fixes from the community.
It may take time to upgrade though, meanwhile, I would suggest you go through the other inconsistecy fixes in 3.6, which might be an issue on 3.4, e.g. ZOOKEEPER-3144 fixed potential global session inconsistency issue, which is a problem on 3.4 as well I believe.
You can check the commit history on 3.6, and see if there are other inconsistency issue might affect your services.
Thank you for your comments ! I got it. |
@lvfangmin Thank you for your comments. We are in the progress of migrating to 3.5. But 3.4 is still in our maintenance list for some elder clusters in the near future. Actually we are reviewing inconsistency issues list in 3.6 commit history. For 3.6, it is not in official release notes. https://zookeeper.apache.org/releases.html. I am not very sure whether it is product ready. But since you have mentioned that FB is already in 3.6, we are considering it's possibility now. |
|
retest maven build |
|
@eolivelli Are u happy to merge this? |
|
retest ant build |
|
@eolivelli PreCommit build on 3.4 is broken due to missing findbugs. |
5 participants
This patch is a backport for the work of ZOOKEEPER-3104. I adapt the original patch to branch-3.4.
Original Author: Allan Lyu fangmin@apache.org
Author: Fangxi Yin yinfangxi@kuaishou.com