[GitHub] [ignite-3] rpuch commented on a diff in pull request #2154: IGNITE-19642 CatalogService should use HybridClock to register new Catalog version

["rpuch (via GitHub)" <gi...@apache.org>]

rpuch commented on code in PR #2154:
URL: https://github.com/apache/ignite-3/pull/2154#discussion_r1221353399


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -519,7 +536,7 @@ public void handle(VersionedUpdate update) {
                 if (entry instanceof NewTableEntry) {
                     catalog = new Catalog(
                             version,
-                            System.currentTimeMillis(),
+                            activationTimestamp(),

Review Comment:
   If I'm not mistaken, `UpdateLog` acts like a WAL, and this code executes the commands contained in the `UpdateLog`.
   
   1. It looks like a command might be executed a few times during the lifetime of the cluster (and probably it will be executed on each node separately), so same schema update might get different activation ts throught its existence. This is not right: each schema update must have an immutable activation moment, and it must be the same on any node of the cluster.
   2. Activation timestamp must be bound to the Metastorage SafeTime; namely, an MS 'message' (from the leader to any cluster node) that transports a schema update must contain the 'message ts' (which, when consumed and fully processed by a node [so, after it makes all modifications to the in-memory Catalog representation], gets assigned to the node's local MS SafeTime variable) and also it must contain the schema update activation ts, and these two timestamps must be connected by the following equality: messageTs+DD=activationTs.
   
   So the activation moment should not be computed here, but in `saveUpdate()` (and it should probably be a part of `VersionedUpdate`).



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org