sashapolo commented on code in PR #2228:
URL: https://github.com/apache/ignite-3/pull/2228#discussion_r1239388840
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/UpdateLog.java:
##########
@@ -67,7 +68,8 @@ interface OnUpdateHandler {
* An actual handler that will be invoked when new update is appended
to the log.
*
* @param update A new update.
+ * @param metastoreUpdateTimestamp Timestamp assigned to the update by
the metastore.
Review Comment:
I'm pretty sure we always use either `Meta Storage` or `Metastorage` when
talking about the Meta Storage, let's not add new terms
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/UpdateLogImpl.java:
##########
@@ -189,14 +193,7 @@ static ByteArray updatePrefix() {
}
}
- private static class UpdateListener implements WatchListener {
Review Comment:
Why did you make this class non-static?
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/UpdateLogImpl.java:
##########
@@ -165,9 +165,13 @@ private void restoreStateFromVault(OnUpdateHandler
handler) {
break;
}
+ if (appliedRevTimestamp == null) {
+ appliedRevTimestamp = metastore.appliedRevisionTimestamp();
+ }
+
VersionedUpdate update = fromBytes(entry.value());
- handler.handle(update);
+ handler.handle(update, appliedRevTimestamp);
Review Comment:
Is this logic correct? We are kind of replaying Catalog updates from the
Vault (by iterating over Catalog versions), is it ok that they will all have
the same timestamp?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/WatchEvent.java:
##########
@@ -33,24 +35,30 @@ public class WatchEvent {
private final long revision;
+ /** Timestamp assigned by the MetaStorage to the event's revision. */
+ private final HybridTimestamp timestamp;
+
/**
* Constructs an watch event with given entry events collection.
*
* @param entryEvts Events for entries corresponding to an update under
one revision.
* @param revision Revision of the updated entries.
+ * @param timestamp Timestamp assigned by the MetaStorage to the event's
revision.
*/
- public WatchEvent(Collection<EntryEvent> entryEvts, long revision) {
+ public WatchEvent(Collection<EntryEvent> entryEvts, long revision,
HybridTimestamp timestamp) {
this.entryEvts = List.copyOf(entryEvts);
this.revision = revision;
+ this.timestamp = timestamp;
}
/**
* Constructs watch event with single entry update.
*
* @param entryEvt Entry event.
*/
+ @TestOnly
public WatchEvent(EntryEvent entryEvt) {
Review Comment:
Let's create a ticket to remove this constructor
--
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]
