sashapolo commented on a change in pull request #402:
URL: https://github.com/apache/ignite-3/pull/402#discussion_r731850355



##########
File path: 
examples/src/test/java/org/apache/ignite/example/table/TableExamplesTest.java
##########
@@ -85,7 +85,7 @@ private void stopNode() {
      */
     @BeforeEach
     @AfterEach
-    private void removeWorkDir() {
+    public void removeWorkDir() {

Review comment:
       Should we delete this method?

##########
File path: 
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -130,7 +130,17 @@ public RocksDbTableStorage(
     }
 
     /** {@inheritDoc} */
-    @Override public void start() throws StorageException {
+    @Override public TableConfiguration configuration() {
+        return tableCfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override public DataRegion dataRegion() {
+        return dataRegion;
+    }
+
+    /** {@inheritDoc} */
+    @Override public synchronized void start() throws StorageException {

Review comment:
       Why is this method synchronized? 

##########
File path: 
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/TableStorage.java
##########
@@ -17,32 +17,77 @@
 
 package org.apache.ignite.internal.storage.engine;
 
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
 import org.apache.ignite.internal.storage.PartitionStorage;
 import org.apache.ignite.internal.storage.StorageException;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Table storage that contains meta, partitions and SQL indexes.
  */
 public interface TableStorage {
     /**
-     * Gets or creates a partition for current table.
+     * Retrieves or creates a partition for the current table. Not expected to 
be called concurrently with the same
+     * partition id.
      *
      * @param partId Partition id.
      * @return Partition storage.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     * @throws StorageException If an error has occurred during the partition 
creation.
      */
-    PartitionStorage getOrCreatePartition(int partId);
+    PartitionStorage getOrCreatePartition(int partId) throws StorageException;
+
+    /**
+     * Returns the partition storage or {@code null} if the requested storage 
doesn't exist.
+     *
+     * @param partId Partition id.
+     * @return Partition storage or {@code null}.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     */
+    @Nullable
+    PartitionStorage getPartition(int partId);
+
+    /**
+     * Destroys a partition if it exists.
+     *
+     * @param partId Partition id.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     * @throws StorageException If an error has occurred during the partition 
destruction.
+     */
+    void dropPartition(int partId) throws StorageException;
+
+    /**
+     * Returns the table configuration.
+     *
+     * @return Table configuration.
+     */
+    TableConfiguration configuration();
+
+    /**
+     * Returns the data region containing table's data.
+     *
+     * @return Data region containing table's data.
+     */
+    DataRegion dataRegion();
 
     /**
      * Starts the storage.
      *
-     * @throws StorageException If something went wrong.
+     * @throws StorageException If error has occurred during the start of the 
storage.

Review comment:
       ```suggestion
        * @throws StorageException If an error has occurred during the start of 
the storage.
   ```

##########
File path: 
examples/src/test/java/org/apache/ignite/example/sql/jdbc/SqlExamplesTest.java
##########
@@ -82,7 +82,7 @@ private void stopNode() {
      */
     @BeforeEach
     @AfterEach
-    private void removeWorkDir() {
+    public void removeWorkDir() {

Review comment:
       Should we delete this method?

##########
File path: 
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -52,9 +52,6 @@
 
 import static java.lang.Integer.parseInt;
 import static java.util.stream.Collectors.groupingBy;
-import static java.util.stream.Stream.concat;
-import static org.apache.ignite.internal.util.CollectionUtils.iteratorToStream;

Review comment:
       Looks like these methods are no longer needed =)

##########
File path: 
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/TableStorage.java
##########
@@ -17,32 +17,77 @@
 
 package org.apache.ignite.internal.storage.engine;
 
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
 import org.apache.ignite.internal.storage.PartitionStorage;
 import org.apache.ignite.internal.storage.StorageException;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Table storage that contains meta, partitions and SQL indexes.
  */
 public interface TableStorage {
     /**
-     * Gets or creates a partition for current table.
+     * Retrieves or creates a partition for the current table. Not expected to 
be called concurrently with the same
+     * partition id.
      *
      * @param partId Partition id.
      * @return Partition storage.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     * @throws StorageException If an error has occurred during the partition 
creation.
      */
-    PartitionStorage getOrCreatePartition(int partId);
+    PartitionStorage getOrCreatePartition(int partId) throws StorageException;
+
+    /**
+     * Returns the partition storage or {@code null} if the requested storage 
doesn't exist.
+     *
+     * @param partId Partition id.
+     * @return Partition storage or {@code null}.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     */
+    @Nullable
+    PartitionStorage getPartition(int partId);
+
+    /**
+     * Destroys a partition if it exists.
+     *
+     * @param partId Partition id.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     * @throws StorageException If an error has occurred during the partition 
destruction.
+     */
+    void dropPartition(int partId) throws StorageException;
+
+    /**
+     * Returns the table configuration.
+     *
+     * @return Table configuration.
+     */
+    TableConfiguration configuration();
+
+    /**
+     * Returns the data region containing table's data.
+     *
+     * @return Data region containing table's data.
+     */
+    DataRegion dataRegion();
 
     /**
      * Starts the storage.
      *
-     * @throws StorageException If something went wrong.
+     * @throws StorageException If error has occurred during the start of the 
storage.
      */
     public void start() throws StorageException;
 
     /**
      * Stops the storage.
      *
-     * @throws StorageException If something went wrong.
+     * @throws StorageException If error has occurred during the stop of the 
storage.
      */
     void stop() throws StorageException;
+
+    /**
+     * Stops and destroys the storage and cleans all allocated resources.
+     *
+     * @throws StorageException If error has occurred during the destruction 
of the storage.

Review comment:
       ```suggestion
        * @throws StorageException If an error has occurred during the 
destruction of the storage.
   ```

##########
File path: 
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/TableStorage.java
##########
@@ -17,32 +17,77 @@
 
 package org.apache.ignite.internal.storage.engine;
 
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
 import org.apache.ignite.internal.storage.PartitionStorage;
 import org.apache.ignite.internal.storage.StorageException;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Table storage that contains meta, partitions and SQL indexes.
  */
 public interface TableStorage {
     /**
-     * Gets or creates a partition for current table.
+     * Retrieves or creates a partition for the current table. Not expected to 
be called concurrently with the same
+     * partition id.
      *
      * @param partId Partition id.
      * @return Partition storage.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     * @throws StorageException If an error has occurred during the partition 
creation.
      */
-    PartitionStorage getOrCreatePartition(int partId);
+    PartitionStorage getOrCreatePartition(int partId) throws StorageException;
+
+    /**
+     * Returns the partition storage or {@code null} if the requested storage 
doesn't exist.
+     *
+     * @param partId Partition id.
+     * @return Partition storage or {@code null}.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     */
+    @Nullable
+    PartitionStorage getPartition(int partId);
+
+    /**
+     * Destroys a partition if it exists.
+     *
+     * @param partId Partition id.
+     * @throws IllegalArgumentException If partition id is out of bounds.
+     * @throws StorageException If an error has occurred during the partition 
destruction.
+     */
+    void dropPartition(int partId) throws StorageException;
+
+    /**
+     * Returns the table configuration.
+     *
+     * @return Table configuration.
+     */
+    TableConfiguration configuration();
+
+    /**
+     * Returns the data region containing table's data.
+     *
+     * @return Data region containing table's data.
+     */
+    DataRegion dataRegion();
 
     /**
      * Starts the storage.
      *
-     * @throws StorageException If something went wrong.
+     * @throws StorageException If error has occurred during the start of the 
storage.
      */
     public void start() throws StorageException;
 
     /**
      * Stops the storage.
      *
-     * @throws StorageException If something went wrong.
+     * @throws StorageException If error has occurred during the stop of the 
storage.

Review comment:
       ```suggestion
        * @throws StorageException If an error has occurred during the stop of 
the storage.
   ```

##########
File path: 
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -130,7 +130,17 @@ public RocksDbTableStorage(
     }
 
     /** {@inheritDoc} */
-    @Override public void start() throws StorageException {
+    @Override public TableConfiguration configuration() {
+        return tableCfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override public DataRegion dataRegion() {
+        return dataRegion;
+    }
+
+    /** {@inheritDoc} */
+    @Override public synchronized void start() throws StorageException {

Review comment:
       Why is this method synchronized? 




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