zookeeper icon indicating copy to clipboard operation
zookeeper copied to clipboard
verified publisher icon apache

ZOOKEEPER-3526: data inconsistency due to mistaken TRUNC caused by maxCommittedLog is much less than minCommittedLog when in readonly mode

Open Hinterwaeldlers opened this issue 5 years ago • 10 comments

Use common zxid between all ZooKeeperServers used within a server instance

Hinterwaeldlers avatar Jun 30 '20 11:06 Hinterwaeldlers

As wished in https://github.com/apache/zookeeper/pull/1391 I've ported the fix to the master branch. After checking the CI result, it seems like it is a general problem with the CI, unrelated to this fix. Right?

Hinterwaeldlers avatar Mar 19 '21 10:03 Hinterwaeldlers

@ztzg @eolivelli @anmolnar @nkalmar This is a serious bug which's related with the consistency, any of your guys have a cycle to review it? @Hinterwaeldlers

maoling avatar Mar 26 '21 04:03 maoling

I hope someone can check this Pull-Request

Hinterwaeldlers avatar Nov 23 '21 07:11 Hinterwaeldlers

@eolivelli you already reviewed the patch for 3.6, are we good to merge this?

nkalmar avatar Nov 23 '21 13:11 nkalmar

@Hinterwaeldlers. Very sorry for our late, the inconsistency issue always has the highest priority. As you see, our reviewer has noticed this PR, I will nudge this PR

maoling avatar Nov 25 '21 04:11 maoling

Any progress within the review progress so far? Thanks in advance

Hinterwaeldlers avatar Feb 07 '22 09:02 Hinterwaeldlers

Oops, I'm looking

maoling avatar Feb 08 '22 02:02 maoling

@Hinterwaeldlers

  • The unit test did not work anymore, please use the Junit5 jupiter.api
  • Please reopen your origin PRs again branch-3.5, branch-3.6, branch-3.7 (Notice master and branch-3.7 use Junit5, branch-3.5 and branch-3.6 use Junit4)

maoling avatar Feb 11 '22 11:02 maoling

Sry for the delay

Currently I am not able to reproduce the bug, as connecting within the RO seems to be impossible in 3.7.1 and 3.8 (using the related docker images). In addition the unit test passes with and without the patch.

While digging around I've found https://github.com/apache/zookeeper/commit/c47ef905e077184bc5b7f555a3e2dfeb6dc046e1 which could fix the problem, too as no session ID is generated anymore.

Related to the RO problem I will create a new bug.

Related to this patch: I think adding only the unit test might be the right way to go, assuming the commit above does fix the zxid usage. I will prepare the PR accordingly

Hinterwaeldlers avatar Sep 01 '22 09:09 Hinterwaeldlers

After I've done a couple of tests and code reading, it seems having the zxid set to the valid value seems still a good thing -> Test + Code adapted accordingly

Hinterwaeldlers avatar Sep 12 '22 12:09 Hinterwaeldlers