tkalkirill commented on code in PR #2254:
URL: https://github.com/apache/ignite-3/pull/2254#discussion_r1279111618


##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -96,414 +86,180 @@ public class IndexManager extends Producer<IndexEvent, 
IndexEventParameters> imp
     /** Table manager. */
     private final TableManager tableManager;
 
+    /** Catalog manager. */
+    private final CatalogManager catalogManager;
+
+    /** Meta storage manager. */
+    private final MetaStorageManager metaStorageManager;
+
     /** Busy lock to stop synchronously. */
     private final IgniteSpinBusyLock busyLock = new IgniteSpinBusyLock();
 
     /** Prevents double stopping of the component. */
     private final AtomicBoolean stopGuard = new AtomicBoolean();
 
+    /** Versioned value used only at the start of the manager. */
+    private final IncrementalVersionedValue<Void> startVv;
+
     /**
      * Constructor.
      *
      * @param tablesCfg Tables and indexes configuration.
      * @param schemaManager Schema manager.
      * @param tableManager Table manager.
+     * @param catalogManager Catalog manager.
      */
     public IndexManager(
             TablesConfiguration tablesCfg,
             SchemaManager schemaManager,
-            TableManager tableManager
+            TableManager tableManager,
+            CatalogManager catalogManager,
+            MetaStorageManager metaStorageManager,
+            Consumer<LongFunction<CompletableFuture<?>>> registry
     ) {
         this.tablesCfg = Objects.requireNonNull(tablesCfg, "tablesCfg");
         this.schemaManager = Objects.requireNonNull(schemaManager, 
"schemaManager");
         this.tableManager = tableManager;
+        this.catalogManager = catalogManager;
+        this.metaStorageManager = metaStorageManager;
+
+        startVv = new IncrementalVersionedValue<>(registry);
     }
 
-    /** {@inheritDoc} */
     @Override
     public void start() {
-        LOG.debug("Index manager is about to start");
-
-        tablesCfg.indexes().listenElements(new ConfigurationListener());
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Index manager is about to start");
+        }
 
-        tableManager.listen(TableEvent.CREATE, (param, ex) -> {
-            if (ex != null) {
-                return completedFuture(false);
-            }
+        startIndexes();
 
-            // We can't return this future as the listener's result, because a 
deadlock can happen in the configuration component:
-            // this listener is called inside a configuration notification 
thread and all notifications are required to finish before
-            // new configuration modifications can occur (i.e. we are creating 
an index below). Therefore we create the index fully
-            // asynchronously and rely on the underlying components to handle 
PK index synchronisation.
-            tableManager.tableAsync(param.causalityToken(), param.tableId())
-                    .thenCompose(table -> {
-                        String[] pkColumns = 
Arrays.stream(table.schemaView().schema().keyColumns().columns())
-                                .map(Column::name)
-                                .toArray(String[]::new);
+        catalogManager.listen(INDEX_CREATE, (parameters, exception) -> {
+            assert exception == null : parameters;
 
-                        String pkName = table.name() + "_PK";
+            return onIndexCreate((CreateIndexEventParameters) parameters);
+        });
 
-                        return createIndexAsync("PUBLIC", pkName, 
table.name(), false,
-                                change -> 
change.changeUniq(true).convert(HashIndexChange.class)
-                                        .changeColumnNames(pkColumns)
-                        );
-                    })
-                    .whenComplete((v, e) -> {
-                        if (e != null) {
-                            LOG.error("Error when creating index: " + e);
-                        }
-                    });
+        catalogManager.listen(INDEX_DROP, (parameters, exception) -> {
+            assert exception == null;
 
-            return completedFuture(false);
+            return onIndexDrop((DropIndexEventParameters) parameters);
         });
 
-        LOG.info("Index manager started");
+        if (LOG.isInfoEnabled()) {
+            LOG.info("Index manager started");
+        }
     }
 
-    /** {@inheritDoc} */
     @Override
     public void stop() throws Exception {
-        LOG.debug("Index manager is about to stop");
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Index manager is about to stop");
+        }

Review Comment:
   Yes, you are right, but there is one caveat, a line will be created in the 
heap that we will not write to the log.
   
   We discussed this issue within the team for a long time, and decided that it 
is better to use such a rule to check through a predicate.
   
   Maybe this should be moved to code style.



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

Reply via email to