tkalkirill commented on code in PR #1619:
URL: https://github.com/apache/ignite-3/pull/1619#discussion_r1095601992
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -137,6 +140,15 @@ public class RocksDbMvPartitionStorage implements
MvPartitionStorage {
/** Maximum size of the key. */
private static final int MAX_KEY_SIZE = ROW_PREFIX_SIZE +
HYBRID_TIMESTAMP_SIZE;
+ /** Garbage collector's queue key's timestamp offset. */
Review Comment:
What lies on the first 2 bytes, please indicate in the documentation or a
constant, it would be convenient for reading to indicate how many bytes in
total.
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -137,6 +140,15 @@ public class RocksDbMvPartitionStorage implements
MvPartitionStorage {
/** Maximum size of the key. */
private static final int MAX_KEY_SIZE = ROW_PREFIX_SIZE +
HYBRID_TIMESTAMP_SIZE;
+ /** Garbage collector's queue key's timestamp offset. */
+ private static final int GC_KEY_TS_OFFSET = Short.BYTES;
+
+ /** Garbage collector's queue key's row id offset. */
+ private static final int GC_KEY_ROW_ID_OFFSET = GC_KEY_TS_OFFSET +
HYBRID_TIMESTAMP_SIZE;
+
+ /** Garbage collector's queue key's size. */
Review Comment:
Same
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -595,21 +619,97 @@ public void addWriteCommitted(RowId rowId, @Nullable
TableRow row, HybridTimesta
@SuppressWarnings("resource") WriteBatchWithIndex writeBatch =
requireWriteBatch();
ByteBuffer keyBuf = prepareHeapKeyBuf(rowId);
- putTimestamp(keyBuf, commitTimestamp);
+ putTimestampDesc(keyBuf, commitTimestamp);
+
+ boolean isNewValueTombstone = row == null;
//TODO IGNITE-16913 Add proper way to write row bytes into array
without allocations.
- byte[] rowBytes = rowBytes(row);
+ byte[] rowBytes = row != null ? rowBytes(row) : new byte[0];
+ boolean newAndPrevTombstones;
try {
- writeBatch.put(cf, copyOf(keyBuf.array(), MAX_KEY_SIZE),
rowBytes);
-
- return null;
+ newAndPrevTombstones = maybeAddToGcQueue(writeBatch, rowId,
commitTimestamp, isNewValueTombstone);
} catch (RocksDBException e) {
- throw new StorageException("Failed to update a row in
storage", e);
+ throw new StorageException("Failed to add row to the GC
queue", e);
+ }
+
+ if (!newAndPrevTombstones) {
+ try {
+ writeBatch.put(cf, copyOf(keyBuf.array(), MAX_KEY_SIZE),
rowBytes);
+ } catch (RocksDBException e) {
+ throw new StorageException("Failed to update a row in
storage", e);
Review Comment:
Please add `RocksDbMvPartitionStorage#createStorageInfo`
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -595,21 +619,97 @@ public void addWriteCommitted(RowId rowId, @Nullable
TableRow row, HybridTimesta
@SuppressWarnings("resource") WriteBatchWithIndex writeBatch =
requireWriteBatch();
ByteBuffer keyBuf = prepareHeapKeyBuf(rowId);
- putTimestamp(keyBuf, commitTimestamp);
+ putTimestampDesc(keyBuf, commitTimestamp);
+
+ boolean isNewValueTombstone = row == null;
//TODO IGNITE-16913 Add proper way to write row bytes into array
without allocations.
- byte[] rowBytes = rowBytes(row);
+ byte[] rowBytes = row != null ? rowBytes(row) : new byte[0];
+ boolean newAndPrevTombstones;
try {
- writeBatch.put(cf, copyOf(keyBuf.array(), MAX_KEY_SIZE),
rowBytes);
-
- return null;
+ newAndPrevTombstones = maybeAddToGcQueue(writeBatch, rowId,
commitTimestamp, isNewValueTombstone);
} catch (RocksDBException e) {
- throw new StorageException("Failed to update a row in
storage", e);
+ throw new StorageException("Failed to add row to the GC
queue", e);
Review Comment:
Please add `RocksDbMvPartitionStorage#createStorageInfo`
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -595,21 +619,97 @@ public void addWriteCommitted(RowId rowId, @Nullable
TableRow row, HybridTimesta
@SuppressWarnings("resource") WriteBatchWithIndex writeBatch =
requireWriteBatch();
ByteBuffer keyBuf = prepareHeapKeyBuf(rowId);
- putTimestamp(keyBuf, commitTimestamp);
+ putTimestampDesc(keyBuf, commitTimestamp);
+
+ boolean isNewValueTombstone = row == null;
//TODO IGNITE-16913 Add proper way to write row bytes into array
without allocations.
- byte[] rowBytes = rowBytes(row);
+ byte[] rowBytes = row != null ? rowBytes(row) : new byte[0];
Review Comment:
Handle `null` - awesome.
Please use `org.apache.ignite.internal.util.ArrayUtils#BYTE_EMPTY_ARRAY`
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -570,17 +588,23 @@ public void commitWrite(RowId rowId, HybridTimestamp
timestamp) throws StorageEx
byte[] valueBytes = writeBatch.getFromBatchAndDB(db, cf,
readOpts, uncommittedKeyBytes);
if (valueBytes == null) {
- //the chain doesn't contain an uncommitted write intent
+ // The chain doesn't contain an uncommitted write intent.
return null;
}
+ boolean isNewValueTombstone = valueBytes.length ==
VALUE_HEADER_SIZE;
+
+ boolean newAndPrevTombstones = maybeAddToGcQueue(writeBatch,
rowId, timestamp, isNewValueTombstone);
Review Comment:
Please give a little comment on the variable, what does it mean?
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -137,6 +140,15 @@ public class RocksDbMvPartitionStorage implements
MvPartitionStorage {
/** Maximum size of the key. */
private static final int MAX_KEY_SIZE = ROW_PREFIX_SIZE +
HYBRID_TIMESTAMP_SIZE;
+ /** Garbage collector's queue key's timestamp offset. */
+ private static final int GC_KEY_TS_OFFSET = Short.BYTES;
+
+ /** Garbage collector's queue key's row id offset. */
Review Comment:
It would be convenient for reading to indicate how many bytes in total.
--
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]
