Phillippko commented on code in PR #4067:
URL: https://github.com/apache/ignite-3/pull/4067#discussion_r1673554590
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/FullStateTransferIndexChooser.java:
##########
@@ -273,137 +261,37 @@ private void onIndexRemoved(RemoveIndexEventParameters
parameters) {
if (catalogVersion <= lwmCatalogVersion) {
// There is no need to add a read-only indexes, since the
index should be destroyed under the updated low watermark.
- tableVersionByIndexId.remove(indexId);
- } else {
- CatalogIndexDescriptor index = indexBusy(indexId,
catalogVersion - 1);
-
- if (index.status() == AVAILABLE) {
- // On drop table event.
- readOnlyIndexes.add(new ReadOnlyIndexInfo(index,
catalogActivationTimestampBusy(catalogVersion), catalogVersion));
- } else if (index.status() == STOPPING) {
- readOnlyIndexes.add(
- new ReadOnlyIndexInfo(index,
findStoppingActivationTsBusy(indexId, catalogVersion - 1), catalogVersion)
- );
- } else {
- // Index that is dropped before even becoming
available.
- tableVersionByIndexId.remove(indexId);
- }
+ return;
}
- });
- });
- }
- private void onIndexCreated(CreateIndexEventParameters parameters) {
- inBusyLock(busyLock, () -> {
- CatalogIndexDescriptor index = parameters.indexDescriptor();
-
- int tableVersion = tableVersionBusy(index,
parameters.catalogVersion());
+ IndexMeta indexMeta = indexMetaStorage.indexMeta(indexId);
- tableVersionByIndexId.put(index.id(), tableVersion);
- });
- }
+ assert indexMeta != null : "indexId=" + indexId + ",
catalogVersion=" + catalogVersion;
- private long catalogActivationTimestampBusy(int catalogVersion) {
- Catalog catalog = catalogService.catalog(catalogVersion);
-
- assert catalog != null : catalogVersion;
-
- return catalog.time();
- }
-
- // TODO: IGNITE-21771 Deal with catalog compaction
- private void recoverStructuresBusy() {
- int earliestCatalogVersion = catalogService.earliestCatalogVersion();
- int latestCatalogVersion = catalogService.latestCatalogVersion();
- int lwmCatalogVersion =
catalogService.activeCatalogVersion(hybridTimestampToLong(lowWatermark.getLowWatermark()));
-
- var tableVersionByIndexId = new HashMap<Integer, Integer>();
- var readOnlyIndexes = new HashSet<ReadOnlyIndexInfo>();
-
- var stoppingActivationTsByIndexId = new HashMap<Integer, Long>();
- var previousCatalogVersionIndexIds = Set.<Integer>of();
-
- for (int catalogVersion = earliestCatalogVersion; catalogVersion <=
latestCatalogVersion; catalogVersion++) {
- int finalCatalogVersion = catalogVersion;
-
- var indexIds = new HashSet<Integer>();
-
- catalogService.indexes(finalCatalogVersion).forEach(index -> {
- tableVersionByIndexId.computeIfAbsent(index.id(), i ->
tableVersionBusy(index, finalCatalogVersion));
-
- if (index.status() == STOPPING) {
- stoppingActivationTsByIndexId.computeIfAbsent(index.id(),
i -> catalogActivationTimestampBusy(finalCatalogVersion));
+ if (indexMeta.status() == MetaIndexStatus.READ_ONLY) {
Review Comment:
I think we could move checking READ_ONLY status to the top, it looks like
the simplest condition to check and the most important
##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -1246,7 +1246,7 @@ public static CompletableFuture<Void>
stopAsync(Supplier<CompletableFuture<Void>
* @param components Array of ignite components to stop.
* @return CompletableFuture that will be completed when all components
are stopped.
*/
- public static CompletableFuture<Void> stopAsync(ComponentContext
componentContext, IgniteComponent... components) {
+ public static CompletableFuture<Void> stopAsync(ComponentContext
componentContext, @Nullable IgniteComponent... components) {
Review Comment:
IMHO nullable is confusing there - it feels like components can be nullable,
and I think we don't have to annotate that varargs can be empty
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/FullStateTransferIndexChooser.java:
##########
@@ -413,17 +301,30 @@ private void
onLwmChanged(ChangeLowWatermarkEventParameters parameters) {
inBusyLock(busyLock, () -> {
int lwmCatalogVersion =
catalogService.activeCatalogVersion(parameters.newLowWatermark().longValue());
- Iterator<ReadOnlyIndexInfo> it = readOnlyIndexes.iterator();
+ readOnlyIndexes.removeIf(readOnlyIndexInfo ->
readOnlyIndexInfo.indexRemovalCatalogVersion() <= lwmCatalogVersion);
+ });
+ }
- while (it.hasNext()) {
- ReadOnlyIndexInfo readOnlyIndexInfo = it.next();
+ private static long activationTs(IndexMeta indexMeta, MetaIndexStatus
status) {
+ return indexMeta.statusChange(status).activationTimestamp();
+ }
- if (readOnlyIndexInfo.indexRemovalCatalogVersion() <=
lwmCatalogVersion) {
- it.remove();
+ private static ReadOnlyIndexInfo toReadOnlyIndexInfo(IndexMeta indexMeta) {
+ assert indexMeta.status() == MetaIndexStatus.READ_ONLY : "indexId=" +
indexMeta.indexId() + ", status=" + indexMeta.status();
- tableVersionByIndexId.remove(readOnlyIndexInfo.indexId());
- }
- }
- });
+ long activationTs;
+
+ if (indexMeta.statusChanges().containsKey(MetaIndexStatus.STOPPING)) {
+ activationTs = activationTs(indexMeta, MetaIndexStatus.STOPPING);
+ } else {
+ activationTs = activationTs(indexMeta, MetaIndexStatus.READ_ONLY);
Review Comment:
Let's add Nullable to indexMeta.statusChange()? javadoc - "Returns the index
status change, {@code null} if absent"
--
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]
