rpuch commented on code in PR #3544:
URL: https://github.com/apache/ignite-3/pull/3544#discussion_r1549829295
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstance.java:
##########
@@ -287,70 +286,95 @@ public ColumnFamily getOrCreateSortedIndexCf(byte[]
cfName, int indexId, int tab
}
/**
- * Schedules a drop of a column family after destroying an index, if it
was the last index managed by that CF.
+ * Removes the given sorted index from this instance. This prevents this
index to be returned by {@link #sortedIndexes} call.
*/
- public CompletableFuture<Void>
scheduleIndexCfsDestroy(List<IndexColumnFamily> indexColumnFamilies) {
- assert !indexColumnFamilies.isEmpty();
+ public void removeSortedIndex(int indexId, ColumnFamily cf) {
+ var cfNameBytes = new ByteArray(cf.nameBytes());
- return flusher.awaitFlush(false)
- .thenRunAsync(() ->
indexColumnFamilies.forEach(this::destroySortedIndexCfIfNeeded),
engine.threadPool());
+ sortedIndexCfsByName.computeIfPresent(cfNameBytes, (unused, indexCf)
-> {
+ indexCf.indexIdToTableId.remove(indexId);
+
+ return indexCf;
+ });
}
- void destroySortedIndexCfIfNeeded(IndexColumnFamily indexColumnFamily) {
- if (!busyLock.enterBusy()) {
- throw new StorageClosedException();
- }
+ /**
+ * Schedules a drop of a column family after destroying an index, if it
was the last index managed by that CF.
+ */
+ public CompletableFuture<Void> scheduleIndexCfsDestroy(List<ColumnFamily>
columnFamilies) {
Review Comment:
This schedules a conditional removal of CFs, but the name does not
communicate this. I was confused when I read this call at its call site. Is it
possible to make it clear from the method name?
##########
modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceTest.java:
##########
@@ -277,6 +281,38 @@ void testConcurrentSortedIndexReadAndCreate() {
assertThat(getIndexFuture.join().stream().map(IndexColumnFamily::indexId).collect(toList()),
contains(0));
}
+ @RepeatedTest(10)
+ void testConcurrentSortedIndexDeleteAndRead() {
+ int tableId = 0;
+
+ int indexId = 0;
+
+ byte[] fooName = sortedIndexCfName(List.of(
+ new StorageSortedIndexColumnDescriptor("a", NativeTypes.INT64,
true, true)
+ ));
+
+ ColumnFamily cf = rocksDb.getOrCreateSortedIndexCf(fooName, indexId,
tableId);
+
+ rocksDb.removeSortedIndex(indexId, cf);
+
+ assertThat(rocksDb.sortedIndexes(tableId), is(empty()));
+ }
+
+ @Test
+ void testDestroyRemovesSortedIndexes() {
Review Comment:
```suggestion
void testTableDestroyRemovesSortedIndexes() {
```
##########
modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceTest.java:
##########
@@ -277,6 +281,38 @@ void testConcurrentSortedIndexReadAndCreate() {
assertThat(getIndexFuture.join().stream().map(IndexColumnFamily::indexId).collect(toList()),
contains(0));
}
+ @RepeatedTest(10)
+ void testConcurrentSortedIndexDeleteAndRead() {
Review Comment:
Where is the concurrency?
--
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]
