kezhuw commented on code in PR #2254:
URL: https://github.com/apache/zookeeper/pull/2254#discussion_r2122595721
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZKDatabase.java:
##########
@@ -320,17 +323,30 @@ public void addCommittedProposal(Request request) {
WriteLock wl = logLock.writeLock();
try {
wl.lock();
- if (committedLog.size() > commitLogCount) {
- committedLog.remove();
- minCommittedLog = committedLog.peek().getZxid();
- }
if (committedLog.isEmpty()) {
minCommittedLog = request.zxid;
maxCommittedLog = request.zxid;
+ } else if (request.zxid <= maxCommittedLog) {
+ // This could happen if lastProcessedZxid is rewinded and
database is re-synced.
+ // Currently, it only happens in test codes, but it should
also be safe for production path.
+ return;
+ } else if (!allowDiscontinuousProposals
+ && request.zxid != maxCommittedLog + 1
+ && ZxidUtils.getEpochFromZxid(request.zxid) <=
ZxidUtils.getEpochFromZxid(maxCommittedLog)) {
+ String msg = String.format(
+ "Committed proposal cached out of order: 0x%s is not the
next proposal of 0x%s",
+ ZxidUtils.zxidToString(request.zxid),
+ ZxidUtils.zxidToString(maxCommittedLog));
+ LOG.error(msg);
+ throw new IllegalStateException(msg);
Review Comment:
I think it is already covered by existing tests.
`allowDiscontinuousProposals`(used by `ZxidRolloverTest`, `TxnLogDigestTest`,
both introduce discontinuous proposals on purpose) was introduced to bypass
this restriction.
Also this path will fail newly introduced test if we rollback changes
involed in sync phase.
```bash
git checkout HEAD^ --
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/
git checkout HEAD^ --
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
```
So, I think it is covered, though not for successful tests.
Besides, this is what I state in commit message.
> Also, this commit rejects discontinuous proposals in syncWithLeader and
committedLog, so to avoid possible data loss.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
