ibessonov commented on a change in pull request #562:
URL: https://github.com/apache/ignite-3/pull/562#discussion_r790565392
##########
File path:
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -254,54 +246,21 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
public PartitionStorage getOrCreatePartition(int partId) throws
StorageException {
- assert !stopped : "Storage has been stopped";
-
- synchronized (partitionsLock) {
- checkPartitionId(partId);
-
- if (partitions[partId] != null) {
- return partitions[partId];
- }
-
- var newPartition = new RocksDbPartitionStorage(threadPool, partId,
db, partitionCf);
-
- partitions[partId] = newPartition;
-
- try {
- meta.putPartitionsList(getPartitionIds());
- } catch (RocksDBException e) {
- throw new StorageException("Unable to save a list of partition
IDs", e);
- }
-
- return newPartition;
- }
- }
+ PartitionStorage storage = getPartition(partId);
- /**
- * Returns a list of existing partition IDs.
- */
- private int[] getPartitionIds() {
- int nonNullPartitionsCount = 0;
-
- for (PartitionStorage s : partitions) {
- if (s != null) {
- nonNullPartitionsCount += 1;
- }
+ if (storage != null) {
+ return storage;
}
- int[] partitionIds = new int[nonNullPartitionsCount];
+ // possible races when creating the partitions with the same ID are
safe, since both the storage creation and the meta update
Review comment:
```suggestion
// Possible races when creating the partitions with the same ID are
safe, since both the storage creation and the meta update
```
##########
File path:
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbPartitionStorage.java
##########
@@ -489,21 +495,12 @@ protected DataRow decodeEntry(byte[] key, byte[] value) {
* and the key's hash (an optimisation).
*/
private byte[] partitionKey(byte[] key) {
- byte[] partitionKey = new byte[PARTITION_KEY_PREFIX_SIZE + key.length];
-
- int pos = 0;
-
- intToBytes(partId, partitionKey, pos, Integer.BYTES);
-
- pos += Integer.BYTES;
-
- intToBytes(Arrays.hashCode(key), partitionKey, pos, Integer.BYTES);
-
- pos += Integer.BYTES;
-
- System.arraycopy(key, 0, partitionKey, pos, key.length);
-
- return partitionKey;
+ return ByteBuffer.allocate(PARTITION_KEY_PREFIX_SIZE + key.length)
+ .order(ByteOrder.BIG_ENDIAN)
+ .putShort((short) partId)
+ .putInt(Arrays.hashCode(key))
Review comment:
I wonder how I missed this last time. Hash must be pre-calculated in
SearchRow as far as I understand. Storage should not control hashing strategy.
Default `Arrays.hashCode` isn't good enough.
--
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]
