ascherbakoff commented on code in PR #4821:
URL: https://github.com/apache/ignite-3/pull/4821#discussion_r1896665963
##########
modules/raft-api/src/main/java/org/apache/ignite/internal/raft/Command.java:
##########
@@ -17,10 +17,27 @@
package org.apache.ignite.internal.raft;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
import org.apache.ignite.internal.network.NetworkMessage;
+import org.jetbrains.annotations.Nullable;
/**
* A marker interface for replication group command.
*/
public interface Command extends NetworkMessage {
+ /**
+ * This is called before a command is submitted to replication pipeline.
+ *
+ * @param safeTs Safe timestamp.
+ */
+ default void patch(HybridTimestamp safeTs) {}
Review Comment:
I think I got you.
Reading safeTime after patching the class in another thread is not thread
safe in the current implementation. This is mutability consequences.
Javadoc on safeTime already mentions special processing for this field:
/** The Safe timestamp. A value for this field is auto generated and should
not be set manually. */
So it should never be read back.
Thread safety issues can be fixed by cloning the command before submitting
it to raft, but IMO doesn't give any profit.
--
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...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
