chihsuan commented on code in PR #10879:
URL: https://github.com/apache/ozone/pull/10879#discussion_r3666922320
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconDBProvider.java:
##########
@@ -111,16 +110,22 @@ public DBStore getDbStore() {
return dbStore;
}
- static void truncateTable(Table table) throws IOException {
+ static <K> void truncateTable(Table<K, ?> table) throws IOException {
if (table == null) {
return;
}
- try (TableIterator<Object, Table.KeyValue<Object, Object>> tableIterator =
table.iterator()) {
- while (tableIterator.hasNext()) {
- KeyValue<Object, Object> entry = tableIterator.next();
- table.delete(entry.getKey());
+ final K firstKey;
+ final K lastKey;
+ try (TableIterator<K, K> keyIterator = table.keyIterator()) {
+ if (!keyIterator.hasNext()) {
+ return;
}
+ firstKey = keyIterator.next();
+ keyIterator.seekToLast();
+ lastKey = keyIterator.next();
}
+ table.deleteRange(firstKey, lastKey);
+ table.delete(lastKey);
Review Comment:
@szetszwo Would it make sense to implement the overload as a default method
in `Table` instead of plumbing it down to the raw layer?
```java
default void deleteRange(KEY beginKey, KEY endKey, boolean includeEndKey)
throws RocksDatabaseException, CodecException {
deleteRange(beginKey, endKey);
if (includeEndKey) {
delete(endKey);
}
}
```
My reasoning, please correct me if I am missing something:
- The existing overrides are reused polymorphically, e.g.
`SchemaOneDeletedBlocksTable` already applies `prefix()` in both methods, so no
per-implementation changes are needed.
- It avoids the successor computation, which is tricky since RocksDB
compares bytes as unsigned: in the sketch, `endKey[i] < Byte.MAX_VALUE` would
treat `0xFF` as incrementable and produce a smaller key.
- The cost is one extra point tombstone, which seems acceptable since
`clear()` discovers the endpoints with an iterator first and is not atomic
either way.
I also plan to handle two related cases explicitly:
- Override `DatanodeTable.clear()` to throw `UnsupportedOperationException`
with a clear message. Iteration is intentionally unsupported there, and
clearing a schema v3 column family could remove data belonging to every
container on the volume.
- Override `InMemoryTestTable.clear()` with `map.clear()`, since its
iterator is also unsupported.
If this direction sounds reasonable, I’ll update the PR with the changes as
separate commits. If a single native `DeleteRange` is preferred, I am happy to
implement it that way too. The main tradeoffs I see are that the overload
becomes abstract in every implementation, and the successor logic depends on
the default bytewise comparator.
--
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]
