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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
