tkalkirill commented on code in PR #1280:
URL: https://github.com/apache/ignite-3/pull/1280#discussion_r1011977990
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshot.java:
##########
@@ -106,12 +110,15 @@ public class OutgoingSnapshot {
*/
private boolean finishedTxData = false;
+ private boolean closed = false;
Review Comment:
Shouldn't it have `volatile`? If I understand correctly, then it should have.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshot.java:
##########
@@ -63,26 +64,29 @@ public class OutgoingSnapshot {
private final PartitionAccess partition;
+ private final LogManager logManager;
+
/**
* Lock that is used for mutual exclusion of MV snapshot reading (by this
class) and threads that write MV data to the same
* partition (currently, via {@link SnapshotAwarePartitionDataStorage}).
*/
private final ReentrantLock mvOperationsLock = new ReentrantLock();
- private final ReusableLockLockup mvOperationsLockup = new
ReusableLockLockup(mvOperationsLock);
+ @Nullable
+ private volatile SnapshotMeta frozenMeta;
Review Comment:
Missing javadoc and explanation why may be `null`.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshot.java:
##########
@@ -133,23 +140,61 @@ public PartitionKey partitionKey() {
}
/**
- * Freezes the scope of this snapshot.
+ * Freezes the scope of this snapshot. This includes taking snapshot
metadata and opening TX data cursor.
*
* <p>Must be called under snapshot lock.
*/
void freezeScope() {
assert mvOperationsLock.isLocked() : "MV operations lock must be
acquired!";
+ frozenMeta = takeSnapshotMeta();
+
txDataCursor = partition.txStatePartitionStorage().scan();
}
+ private SnapshotMeta takeSnapshotMeta() {
+ long lastAppliedIndex = Math.max(
+ partition.mvPartitionStorage().lastAppliedIndex(),
+ partition.txStatePartitionStorage().lastAppliedIndex()
+ );
+
+ return SnapshotMetaUtils.snapshotMetaAt(lastAppliedIndex, logManager);
+ }
+
+ /**
+ * Returns metadata corresponding to this snapshot.
+ *
+ * @return This snapshot metadata.
+ */
+ public SnapshotMeta meta() {
+ SnapshotMeta meta = frozenMeta;
+
+ assert meta != null : "No snapshot meta yet, probably the snapshot
scope was not yet frozen";
+
+ return meta;
+ }
+
/**
- * Reads a snapshot meta and returns a future with the response.
+ * Reads the snapshot meta and returns a future with the response.
*
- * @param metaRequest Meta request.
+ * @param request Meta request.
*/
- SnapshotMetaResponse handleSnapshotMetaRequest(SnapshotMetaRequest
metaRequest) {
- //TODO https://issues.apache.org/jira/browse/IGNITE-17935
+ @Nullable
+ SnapshotMetaResponse handleSnapshotMetaRequest(SnapshotMetaRequest
request) {
+ if (closed) {
+ return logAlreadyClosedAndReturnNull();
+ }
+
+ SnapshotMeta meta = frozenMeta;
+
+ assert meta != null : "No snapshot meta yet, probably the snapshot
scope was not yet frozen";
+
+ return MESSAGES_FACTORY.snapshotMetaResponse().meta(meta).build();
+ }
+
+ @Nullable
+ private <T> T logAlreadyClosedAndReturnNull() {
Review Comment:
Missing the assertion that the method was called by `closed`.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/SnapshotMetaUtils.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.table.distributed.raft.snapshot.outgoing;
+
+import static java.util.stream.Collectors.toList;
+
+import java.util.Collection;
+import java.util.List;
+import org.apache.ignite.raft.jraft.RaftMessagesFactory;
+import org.apache.ignite.raft.jraft.conf.ConfigurationEntry;
+import org.apache.ignite.raft.jraft.entity.PeerId;
+import org.apache.ignite.raft.jraft.entity.RaftOutter.SnapshotMeta;
+import org.apache.ignite.raft.jraft.entity.SnapshotMetaBuilder;
+import org.apache.ignite.raft.jraft.storage.LogManager;
+
+/**
+ * Utils to build {@link SnapshotMeta} instances.
+ */
+public class SnapshotMetaUtils {
+ /**
+ * Builds a {@link SnapshotMeta} corresponding to RAFT state (term,
configuration) at the given log index.
+ *
+ * @param logIndex RAFT log index.
+ * @param logManager LogManager from which to load term and configuration.
Review Comment:
Which LogManager?
--
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]
