ibessonov commented on code in PR #1246:
URL: https://github.com/apache/ignite-3/pull/1246#discussion_r1007774920
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshotsManager.java:
##########
@@ -103,17 +103,17 @@ public void stop() throws Exception {
}
/**
- * Registers an outgoing snapshot in the manager.
+ * Starts an outgoing snapshot and registers it in the manager. This is
the point where snapshot is 'taken',
+ * that is, the immutable scope of the snapshot (what MV data and what TX
data belongs to it) is cut.
*
* @param snapshotId Snapshot id.
* @param outgoingSnapshot Outgoing snapshot.
*/
- @Override
- public void registerOutgoingSnapshot(UUID snapshotId, OutgoingSnapshot
outgoingSnapshot) {
+ void startOutgoingSnapshot(UUID snapshotId, OutgoingSnapshot
outgoingSnapshot) {
snapshots.put(snapshotId, outgoingSnapshot);
PartitionSnapshotsImpl partitionSnapshots =
getPartitionSnapshots(outgoingSnapshot.partitionKey());
Review Comment:
I don't like using specific implementation when it's not required. It's like
writing `ArrayList<?> list = new ArrayList<>()`, just unnecessary, unless you
call something like `trimToSize`, which is a very specific operation
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshot.java:
##########
@@ -124,63 +132,62 @@ public PartitionKey partitionKey() {
return partition.partitionKey();
}
+ /**
+ * Freezes the scope of this snapshot.
+ *
+ * <p>Must be called under snapshot lock.
+ */
+ void freezeScope() {
+ throwIfMvLockNotAcquired();
Review Comment:
Isn't this supposed to be an assertion?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxStateStorage.java:
##########
@@ -63,7 +64,7 @@ public interface TxStateStorage extends AutoCloseable {
* @throws IgniteInternalException with {@link
Transactions#TX_STATE_STORAGE_ERR} error code in case when
* the operation has failed.
*/
- boolean compareAndSet(UUID txId, TxState txStateExpected, TxMeta txMeta,
long commandIndex);
+ boolean compareAndSet(UUID txId, @Nullable TxState txStateExpected, TxMeta
txMeta, long commandIndex);
Review Comment:
Didn't know that, thank you!
##########
modules/transactions/src/testFixtures/java/org/apache/ignite/internal/tx/storage/state/test/TestTxStateStorage.java:
##########
@@ -82,7 +85,11 @@ public void remove(UUID txId) {
@Override
public Cursor<IgniteBiTuple<UUID, TxMeta>> scan() {
- return Cursor.fromIterator(storage.entrySet().stream().map(e -> new
IgniteBiTuple<>(e.getKey(), e.getValue())).iterator());
+ List<IgniteBiTuple<UUID, TxMeta>> copy = storage.entrySet().stream()
+ .map(e -> new IgniteBiTuple<>(e.getKey(), e.getValue()))
+ .collect(toList());
+
+ return Cursor.fromIterator(copy.iterator());
Review Comment:
I think now we have `fromIterable`, but this is up to you
--
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]
