yifan-c commented on code in PR #333:
URL: https://github.com/apache/cassandra-sidecar/pull/333#discussion_r3229953816
##########
adapters/adapters-base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CassandraTableOperations.java:
##########
@@ -65,6 +65,30 @@ public List<String> importNewSSTables(@NotNull String
keyspace,
copyData);
}
+ /**
+ * {@inheritDoc}
+ * <p>
+ * SAI index validation is only supported in Cassandra 5.0+; for older
versions these parameters are ignored.
+ */
+ @Override
+ public List<String> importNewSSTables(@NotNull String keyspace,
+ @NotNull String tableName,
+ @NotNull String directory,
+ boolean resetLevel,
+ boolean clearRepaired,
+ boolean verifySSTables,
+ boolean verifyTokens,
+ boolean invalidateCaches,
+ boolean extendedVerify,
+ boolean copyData,
+ boolean failOnMissingIndex,
+ boolean validateIndexChecksum)
+ {
+ return importNewSSTables(keyspace, tableName, directory, resetLevel,
+ clearRepaired, verifySSTables, verifyTokens,
+ invalidateCaches, extendedVerify, copyData);
+ }
Review Comment:
indentation
```suggestion
return importNewSSTables(keyspace, tableName, directory, resetLevel,
clearRepaired, verifySSTables, verifyTokens,
invalidateCaches, extendedVerify, copyData);
```
##########
server/src/testFixtures/java/org/apache/cassandra/sidecar/restore/RestoreJobTestUtils.java:
##########
Review Comment:
Why is it moved from test scope to testFixture scope? The relocation leads
to the textFixture depndencies update in `server/build.gradle`
##########
server/src/main/java/org/apache/cassandra/sidecar/utils/SSTableImporter.java:
##########
@@ -248,8 +253,7 @@ private void drainImportQueue(ImportQueue queue)
successCount++;
LOGGER.info("Successfully imported SSTables with
options={}, serviceTimeMillis={}",
options,
TimeUnit.NANOSECONDS.toMillis(serviceTimeNanos));
- promise.complete();
- cleanup(options);
+ cleanup(options).onComplete(ar -> promise.complete());
Review Comment:
This changes the behavior of the `SSTableImporter`.
Previously, cleanup is a best-effort; the cleanup can fail and operators can
recover manually. Now, if cleanup fails, import fails.
I do not believe that is the desired behavior of the sstable importing.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
