SammyVimes commented on code in PR #1246:
URL: https://github.com/apache/ignite-3/pull/1246#discussion_r1005431722
##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -626,6 +626,19 @@ public static void closeAll(AutoCloseable... closeables)
throws Exception {
closeAll(Arrays.stream(closeables));
}
+ /**
+ * Closes an {@link AutoCloseable} ignoring any exception it throws.
+ *
+ * @param closeable The AutoCloseable to close.
+ */
+ public static void closeQuietly(AutoCloseable closeable) {
+ try {
+ closeable.close();
+ } catch (Exception e) {
+ // Ignore.
Review Comment:
Why do we want to close something quietly?
##########
modules/core/src/main/java/org/apache/ignite/internal/lock/ReusableLockLockup.java:
##########
@@ -29,15 +29,20 @@
public class ReusableLockLockup implements AutoLockup {
private final Lock lock;
- public ReusableLockLockup(Lock lock) {
+ private ReusableLockLockup(Lock lock) {
Review Comment:
Will there be other implementations? Seems like we can have just this one
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshot.java:
##########
@@ -133,49 +155,35 @@ CompletableFuture<SnapshotMetaResponse>
handleSnapshotMetaRequest(SnapshotMetaRe
}
/**
- * Reads chunk of partition data and returns a future with the response.
+ * Reads a chunk of partition data and returns a future with the response.
*
* @param request Data request.
*/
CompletableFuture<SnapshotMvDataResponse>
handleSnapshotMvDataRequest(SnapshotMvDataRequest request) {
- // TODO: IGNITE-17935 - executor?
-
- assert !finished;
+ assert !finishedMvData : "MV data sending has already been finished";
Review Comment:
Why return future if we have the data?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshot.java:
##########
@@ -220,10 +228,10 @@ private long
tryProcessRowFromPartition(List<SnapshotMvDataResponse.ResponseEntr
return totalBatchSize;
}
- if (!startedToReadPartition) {
+ if (!startedToReadMvPartition) {
Review Comment:
I thought we are going away from `MV` prefix since we only have `MV`
storages?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshot.java:
##########
@@ -122,6 +135,15 @@ public PartitionKey partitionKey() {
return partition.key();
}
+ /**
+ * Freezes the scope of this snapshot.
+ *
+ * <p>Must be called under snapshot lock.
+ */
+ void freezeScope() {
+ txDataCursor = partition.scanTxData();
Review Comment:
Can we check that the snapshot lock is held here?
--
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]
