xtern commented on code in PR #4256:
URL: https://github.com/apache/ignite-3/pull/4256#discussion_r1730837941
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java:
##########
@@ -704,6 +720,10 @@ private void
handleUpdateMinimalActiveTxTimeCommand(UpdateMinimumActiveTxBeginTi
assert minActiveTxBeginTime0 <= cmd.timestamp() : "maxTime=" +
minActiveTxBeginTime0 + ", cmdTime=" + cmd.timestamp();
minActiveTxBeginTime = cmd.timestamp();
+
+ storage.flush(false).whenComplete((r, t) -> {
+ snapshottedMinActiveTxBeginTime = minActiveTxBeginTime;
Review Comment:
flush(false) should guarantee us that if a checkpoint has already started at
the moment, then we “subscribe” to the next one.
with this implementation
```
snapshottedMinActiveTxBeginTime = minActiveTxBeginTime;
```
the following situation may occur
1. we subscribed to a checkpoint (minActiveTxBeginTime=N)
2. the checkpoint started
3. a new value arrived minActiveTxBeginTime = N + M
4. we subscribe to the next checkpoint
5. the current checkpoint ends but we write not N but immediately N + M,
which can be a problem
Thus, I suggest to rework this a bit.
1. whenComplete listener must use some local value instead of
minActiveTxBeginTime (may be we can completely remove minActiveTxBeginTime
field).
2. should update snapshottedMinActiveTxBeginTime only if local value is
greater then stored in snapshottedMinActiveTxBeginTime (because
completablefuture listeners are called in reverse order)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
