alievmirza commented on code in PR #6651:
URL: https://github.com/apache/ignite-3/pull/6651#discussion_r2391272374
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DataNodesManager.java:
##########
@@ -1042,18 +1070,23 @@ private CompletableFuture<Void> msInvokeWithRetry(
CompletableFuture<DataNodesHistoryMetaStorageOperation>
> metaStorageOperationSupplier,
int attemptsLeft,
- CatalogZoneDescriptor zone
+ CatalogZoneDescriptor zone,
+ boolean ensureContextIsPresent
) {
if (attemptsLeft <= 0) {
throw new AssertionError("Failed to perform meta storage invoke,
maximum number of attempts reached [zone=" + zone + "].");
}
// Get locally on the first attempt, otherwise it means that invoke
has failed because of different value in meta storage,
// so we need to retrieve the value from meta storage.
- DataNodeHistoryContextMetaStorageGetter msGetter = attemptsLeft ==
MAX_ATTEMPTS_ON_RETRY
+ DataNodeHistoryContextMetaStorageGetter msGetter0 = attemptsLeft ==
MAX_ATTEMPTS_ON_RETRY
? this::getDataNodeHistoryContextMsLocally
: this::getDataNodeHistoryContextMs;
+ DataNodeHistoryContextMetaStorageGetter msGetter =
ensureContextIsPresent
+ ? keys -> msGetter0.get(keys).thenCompose(context ->
ensureContextIsPresent(context, keys, zone.id()))
Review Comment:
We have discussed in private and agreed that approach must be changed. This
is the example when current solution won't work:
1. Stop all cluster
2. Update all nodes version
3. Start new cluster
4. None of the listed actions happen in the cluster:
filter/replica/autoadjust value change
5. In that case context won't be initialised
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DataNodesManager.java:
##########
@@ -1306,6 +1360,18 @@ private static CompletableFuture<Void> processWatchEvent(
return nullCompletedFuture();
}
+ private CatalogZoneDescriptor zoneDescriptor(int zoneId) {
+ int catalogVersion =
catalogManager.activeCatalogVersion(clockService.currentLong());
Review Comment:
why not `catalogManager.latestCatalogVersion`?
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DataNodesManager.java:
##########
@@ -992,6 +1002,21 @@ private CompletableFuture<DataNodesHistoryContext>
getDataNodeHistoryContextMsLo
return completedFuture(dataNodeHistoryContextFromValues(entries));
}
+ private CompletableFuture<DataNodesHistoryContext> ensureContextIsPresent(
Review Comment:
let's rename it and reflect the fact that this method also changes the state
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DataNodesManager.java:
##########
@@ -1306,6 +1360,18 @@ private static CompletableFuture<Void> processWatchEvent(
return nullCompletedFuture();
}
+ private CatalogZoneDescriptor zoneDescriptor(int zoneId) {
+ int catalogVersion =
catalogManager.activeCatalogVersion(clockService.currentLong());
Review Comment:
why `clockService.currentLong()` but not `clockService.nowLong()`
--
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
