korlov42 commented on code in PR #5112:
URL: https://github.com/apache/ignite-3/pull/5112#discussion_r1930076137
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/Catalog.java:
##########
@@ -46,7 +47,8 @@
import org.jetbrains.annotations.Nullable;
/**
- * Catalog descriptor represents database schema snapshot.
+ * Catalog descriptor represents a snapshot of the database schema.
+ * It contains information about schemas, tables, indexes, and zones available
in the current version of the catalog.
Review Comment:
```suggestion
* Catalog descriptor represents a snapshot of the database schema.
*
* <p>It contains information about schemas, tables, indexes, and zones
available in the current version of the catalog.
```
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogService.java:
##########
@@ -63,83 +56,49 @@ public interface CatalogService extends
EventProducer<CatalogEvent, CatalogEvent
/** Default storage profile. */
String DEFAULT_STORAGE_PROFILE = "default";
- @Nullable Catalog catalog(int catalogVersion);
-
/**
- * Returns table descriptor by the given schema name and table name at
given timestamp.
+ * Retrieves the catalog for the specified version.
*
- * @return Table descriptor or {@code null} if table not found.
+ * @param catalogVersion The version of the catalog to retrieve.
+ * @return The catalog for the specified version, or {@code null} if not
found.
*/
- @Nullable CatalogTableDescriptor table(String schemaName, String
tableName, long timestamp);
+ @Nullable Catalog catalog(int catalogVersion);
/**
- * Returns table descriptor by the given table ID and given timestamp.
+ * Retrieves the catalog, which was actual at the specified timestamp.
*
- * @return Table descriptor or {@code null} if table not found.
+ * @param timestamp The point-in-time to retrieve the catalog of actual
version.
+ * @return The active catalog at the specified timestamp.
*/
- @Nullable CatalogTableDescriptor table(int tableId, long timestamp);
+ Catalog activeCatalog(long timestamp);
/**
- * Returns table descriptor by the given table ID and catalog version.
+ * Retrieves the actual catalog version at the specified timestamp.
*
- * @return Table descriptor or {@code null} if table not found.
+ * @param timestamp The point-in-time to retrieve the actual catalog
version.
+ * @return The active catalog version at the specified timestamp.
*/
- @Nullable CatalogTableDescriptor table(int tableId, int catalogVersion);
-
- Collection<CatalogTableDescriptor> tables(int catalogVersion);
+ int activeCatalogVersion(long timestamp);
/**
- * Returns a descriptor for <em>alive</em> index by the given schema name
and index name at given timestamp,
- * that is an index that has not been dropped yet at a given point in time.
+ * Returns the earliest registered version of the catalog.
*
- * <p>This effectively means that the index must be present in the Catalog
and not in the {@link CatalogIndexStatus#STOPPING}
- * state.
+ * @return The earliest registered version of the catalog.
*/
- @Nullable CatalogIndexDescriptor aliveIndex(String schemaName, String
indexName, long timestamp);
-
- @Nullable CatalogIndexDescriptor index(int indexId, long timestamp);
+ int earliestCatalogVersion();
/**
- * Returns index descriptor by the given index ID and catalog version.
+ * Returns the latest registered version of the catalog.
*
- * @return Index descriptor or {@code null} if index not found.
+ * @return The latest registered version of the catalog.
*/
- @Nullable CatalogIndexDescriptor index(int indexId, int catalogVersion);
-
- Collection<CatalogIndexDescriptor> indexes(int catalogVersion);
-
- List<CatalogIndexDescriptor> indexes(int catalogVersion, int tableId);
-
- @Nullable CatalogSchemaDescriptor schema(int catalogVersion);
-
- @Nullable CatalogSchemaDescriptor schema(@Nullable String schemaName, int
catalogVersion);
-
- @Nullable CatalogSchemaDescriptor schema(int schemaId, int catalogVersion);
-
- @Nullable CatalogZoneDescriptor zone(String zoneName, long timestamp);
-
- @Nullable CatalogZoneDescriptor zone(int zoneId, long timestamp);
-
- @Nullable CatalogZoneDescriptor zone(int zoneId, int catalogVersion);
-
- Collection<CatalogZoneDescriptor> zones(int catalogVersion);
-
- @Nullable CatalogSchemaDescriptor activeSchema(long timestamp);
-
- @Nullable CatalogSchemaDescriptor activeSchema(@Nullable String
schemaName, long timestamp);
-
- int activeCatalogVersion(long timestamp);
-
- /** Returns the earliest registered version of the catalog. */
- int earliestCatalogVersion();
-
- /** Returns the latest registered version of the catalog. */
int latestCatalogVersion();
/**
* Returns a future, which completes, when catalog of given version will
be available.
*
- * @param version Catalog version to wait for.
+ * @param version The catalog version to wait for.
+ * @return A future that completes when the catalog of the given version
becomes available.
*/
CompletableFuture<Void> catalogReadyFuture(int version);
Review Comment:
perhaps, it worth to move `catalogInitializationFuture` method to
`CatalogManager` interface or even implementation class. WDYT?
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogService.java:
##########
@@ -63,83 +56,49 @@ public interface CatalogService extends
EventProducer<CatalogEvent, CatalogEvent
/** Default storage profile. */
String DEFAULT_STORAGE_PROFILE = "default";
- @Nullable Catalog catalog(int catalogVersion);
-
/**
- * Returns table descriptor by the given schema name and table name at
given timestamp.
+ * Retrieves the catalog for the specified version.
Review Comment:
```suggestion
* Retrieves the catalog of the specified version.
```
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogService.java:
##########
@@ -42,7 +34,8 @@
* <li>Events are fired in the metastore thread.</li>
* </ul>
*
- * <p>TBD: events
+ * @see CatalogEvent Full list of events, which is fired by the catalog
service.
+ * @see CatalogManager The manager, which provides catalog manipulation
methods and is responsible for managing distributed operations.
*/
public interface CatalogService extends EventProducer<CatalogEvent,
CatalogEventParameters> {
Review Comment:
let's improve javadoc a bit:
- for `@Nullable Catalog catalog(int catalogVersion)` let's mention in what
cases it will return `null`. I believe it's either version has sunk under
watermark, or catalog update has not been processed yet. In case of former
there is nothing we can do, but in case of latter the caller side may guard
invocation with awaiting of version to be ready (here it would be nice to
provide link to `catalogReadyFuture` method)
- for `Catalog activeCatalog(long timestamp);` let's mention, the same
invocation, technically, may return different version of catalog, and it's up
caller side to make sure that timestamp is less than node's safe time
- for `int earliestCatalogVersion()` let's mentioned, that it's earliest
_available_ version at the moment, and garbage collector disposes earliest
versions from time to time
- `int latestCatalogVersion();` -- I believe this is the most tricky one. A
latest doesn't mean active. Moreover, latest doesn't mean fully processed. It's
worth to mention, that there is no guarantee, that all components have seen and
processed all events related to this version. To make it safe to use this
version across all the components, it must be guarded with `catalogReadyFuture`
call
- `CompletableFuture<Void> catalogReadyFuture(int version);` -- just throw a
few lines while it's crucial to wait for catalog readiness (see previous item)
--
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
