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


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableUtils.java:
##########
@@ -81,4 +87,44 @@ public static int 
findStartBuildingIndexCatalogVersion(CatalogService catalogSer
                 BUILDING, indexId, fromCatalogVersionIncluded, 
latestCatalogVersion
         ));
     }
+
+    /**
+     * Collects a list of tables that were removed from the catalog and should 
have been dropped due to a low watermark (if the catalog
+     * version in which the table was removed is less than or equal to the 
active catalog version of low watermark).
+     *
+     * @param catalogService Catalog service.
+     * @param lowWatermark Low watermark, {@code null} if it has never been 
updated.
+     * @return Result is sorted by the catalog version in which the table was 
removed from the catalog and by the table ID.
+     */
+    // TODO: IGNITE-21771 Process or check catalog compaction
+    static List<DroppedTableInfo> droppedTables(CatalogService catalogService, 
@Nullable HybridTimestamp lowWatermark) {

Review Comment:
   > How is this going to work with catalog compaction?
   Since we won’t be compacting the catalog at the moment, we don’t need to 
worry about this for now. 
   In order not to forget about this above, I added IGNITE-21771 so as not to 
forget about it. I think that the compaction of the catalog will be done in the 
form of a separate epic, everything needs to be done there correctly.
   
   > Why don't we use the same approach as we use in indexes, i.e. check all 
existing tables in the storage and remove those which have been covered by the 
low watermark?
   Tables and indexes are still different, I don’t quite understand your idea, 
for example, we have different engines where tables can be stored and we do not 
raise them when the engines start, but lazily load them. Can you give your 
solution more specifically and prove that it is better or faster?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableUtils.java:
##########
@@ -81,4 +87,44 @@ public static int 
findStartBuildingIndexCatalogVersion(CatalogService catalogSer
                 BUILDING, indexId, fromCatalogVersionIncluded, 
latestCatalogVersion
         ));
     }
+
+    /**
+     * Collects a list of tables that were removed from the catalog and should 
have been dropped due to a low watermark (if the catalog
+     * version in which the table was removed is less than or equal to the 
active catalog version of low watermark).
+     *
+     * @param catalogService Catalog service.
+     * @param lowWatermark Low watermark, {@code null} if it has never been 
updated.
+     * @return Result is sorted by the catalog version in which the table was 
removed from the catalog and by the table ID.
+     */
+    // TODO: IGNITE-21771 Process or check catalog compaction
+    static List<DroppedTableInfo> droppedTables(CatalogService catalogService, 
@Nullable HybridTimestamp lowWatermark) {

Review Comment:
   > How is this going to work with catalog compaction?
   
   Since we won’t be compacting the catalog at the moment, we don’t need to 
worry about this for now. 
   In order not to forget about this above, I added IGNITE-21771 so as not to 
forget about it. I think that the compaction of the catalog will be done in the 
form of a separate epic, everything needs to be done there correctly.
   
   > Why don't we use the same approach as we use in indexes, i.e. check all 
existing tables in the storage and remove those which have been covered by the 
low watermark?
   
   Tables and indexes are still different, I don’t quite understand your idea, 
for example, we have different engines where tables can be stored and we do not 
raise them when the engines start, but lazily load them. Can you give your 
solution more specifically and prove that it is better or faster?



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