[GitHub] [ignite-3] denis-chudov commented on a diff in pull request #2500: IGNITE-19499 TableManager should listen CatalogService events instead of configuration

[via GitHub]

denis-chudov commented on code in PR #2500:
URL: https://github.com/apache/ignite-3/pull/2500#discussion_r1318544220


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2740,47 +2255,47 @@ private PartitionUpdateHandlers 
createPartitionUpdateHandlers(
         return findTableImplByName(startedTables.values(), name);
     }
 
-    private @Nullable CatalogTableDescriptor getTableDescriptor(int id) {
-        TableView tableView = findTableView(tablesCfg.value(), id);
+    private CatalogTableDescriptor getTableDescriptor(int tableId, int 
catalogVersion) {
+        CatalogTableDescriptor tableDescriptor = catalogService.table(tableId, 
catalogVersion);
+
+        assert tableDescriptor != null : "tableId=" + tableId + ", 
catalogVersion=" + catalogVersion;
 
-        return tableView == null ? null : toTableDescriptor(tableView);
+        return tableDescriptor;
     }
 
-    private @Nullable CatalogZoneDescriptor getZoneDescriptor(int zoneId, int 
catalogVersion) {
-        return catalogService.zone(zoneId, catalogVersion);
+    private CatalogZoneDescriptor getZoneDescriptor(CatalogTableDescriptor 
tableDescriptor, int catalogVersion) {
+        CatalogZoneDescriptor zoneDescriptor = 
catalogService.zone(tableDescriptor.zoneId(), catalogVersion);
+
+        assert zoneDescriptor != null :
+                "tableId=" + tableDescriptor.id() + ", zoneId=" + 
tableDescriptor.zoneId() + ", catalogVersion=" + catalogVersion;
+
+        return zoneDescriptor;
     }
 
     private static @Nullable TableImpl 
findTableImplByName(Collection<TableImpl> tables, String name) {
         return tables.stream().filter(table -> 
table.name().equals(name)).findAny().orElse(null);
     }
 
-    /**
-     * Fires table creation events so that indexes can be correctly created at 
IndexManager startup.
-     *
-     * <p>NOTE: This is a temporary solution that must be get 
rid/remake/change.
-     */
-    // TODO: IGNITE-19499 Need to get rid/remake/change
-    private void fireCreateTablesOnManagerStart() {
+    private void startTables() {
         CompletableFuture<Long> recoveryFinishFuture = 
metaStorageMgr.recoveryFinishedFuture();
 
         assert recoveryFinishFuture.isDone();
 
+        int catalogVersion = catalogService.latestCatalogVersion();
         long causalityToken = recoveryFinishFuture.join();
 
-        List<CompletableFuture<?>> fireEventFutures = new ArrayList<>();
+        List<CompletableFuture<?>> startTableFutures = new ArrayList<>();
 
-        for (TableView tableView : tablesCfg.tables().value()) {
-            fireEventFutures.add(fireEvent(TableEvent.CREATE, new 
TableEventParameters(causalityToken, tableView.id())));
+        for (CatalogTableDescriptor tableDescriptor : 
catalogService.tables(catalogVersion)) {
+            startTableFutures.add(createTableLocally(causalityToken, 
catalogVersion, tableDescriptor));
         }
 
-        startVv.update(causalityToken, (unused, throwable) -> 
allOf(fireEventFutures.toArray(CompletableFuture[]::new)))
+        startVv.update(causalityToken, (unused, throwable) -> 
allOf(startTableFutures.toArray(CompletableFuture[]::new)))

Review Comment:
   I am not sure that I understand the point of `startVv` here.  Looks like 
versioned value is not really needed, completable future would be ok.



-- 
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