You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2020/07/25 06:48:38 UTC
[GitHub] [incubator-iotdb] neuyilan opened a new pull request #1560: [Distributed] bug fix: Leader cannot be found
neuyilan opened a new pull request #1560:
URL: https://github.com/apache/incubator-iotdb/pull/1560
1. bug fix, seperate the tsfile and timeseriesschema collection in snapshot task
2.bug fix: Shrink the lock of the logManager in applyPartitionedSnapshot
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-iotdb] neuyilan commented on a change in pull request #1560: [Distributed] bug fix: Leader cannot be found
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #1560:
URL: https://github.com/apache/incubator-iotdb/pull/1560#discussion_r460611000
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/manage/FilePartitionedSnapshotLogManager.java
##########
@@ -52,21 +54,44 @@ public FilePartitionedSnapshotLogManager(LogApplier logApplier, PartitionTable p
@Override
public void takeSnapshot() throws IOException {
-
logger.info("Taking snapshots, flushing IoTDB");
StorageEngine.getInstance().syncCloseAllProcessor();
logger.info("Taking snapshots, IoTDB is flushed");
- synchronized (slotSnapshots) {
+ // TODO-cluster https://issues.apache.org/jira/browse/IOTDB-820
+ synchronized (this) {
collectTimeseriesSchemas();
snapshotLastLogIndex = getCommitLogIndex();
snapshotLastLogTerm = getCommitLogTerm();
- collectTsFiles();
+ collectTsFilesAndFillTimeseriesSchemas();
logger.info("Snapshot is taken");
}
}
- private void collectTsFiles() throws IOException {
+ /**
+ * IMPORTANT, separate the collection timeseries schema from tsfile to avoid the following
+ * situations: If the tsfile is empty at this time (only the metadata is registered, but the
+ * tsfile has not been written yet), then the timeseries schema snapshot can still be generated
+ * and sent to the followers.
+ *
+ * @throws IOException
+ */
+ private void collectTsFilesAndFillTimeseriesSchemas() throws IOException {
+ // 1.collect tsfile
+ collectTsFiles();
+
+ //2.register the measurement
+ for (Map.Entry<Integer, Collection<TimeseriesSchema>> entry : slotTimeseries.entrySet()) {
+ int slotNum = entry.getKey();
+ FileSnapshot snapshot = slotSnapshots.computeIfAbsent(slotNum,
+ s -> new FileSnapshot());
+ if (snapshot.getTimeseriesSchemas().isEmpty()) {
+ snapshot.setTimeseriesSchemas(slotTimeseries.getOrDefault(slotNum,
+ Collections.emptySet()));
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #1560: [Distributed] bug fix: Leader cannot be found
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #1560:
URL: https://github.com/apache/incubator-iotdb/pull/1560#discussion_r460602437
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/manage/FilePartitionedSnapshotLogManager.java
##########
@@ -52,21 +54,44 @@ public FilePartitionedSnapshotLogManager(LogApplier logApplier, PartitionTable p
@Override
public void takeSnapshot() throws IOException {
-
logger.info("Taking snapshots, flushing IoTDB");
StorageEngine.getInstance().syncCloseAllProcessor();
logger.info("Taking snapshots, IoTDB is flushed");
- synchronized (slotSnapshots) {
+ // TODO-cluster https://issues.apache.org/jira/browse/IOTDB-820
+ synchronized (this) {
collectTimeseriesSchemas();
snapshotLastLogIndex = getCommitLogIndex();
snapshotLastLogTerm = getCommitLogTerm();
- collectTsFiles();
+ collectTsFilesAndFillTimeseriesSchemas();
logger.info("Snapshot is taken");
}
}
- private void collectTsFiles() throws IOException {
+ /**
+ * IMPORTANT, separate the collection timeseries schema from tsfile to avoid the following
+ * situations: If the tsfile is empty at this time (only the metadata is registered, but the
+ * tsfile has not been written yet), then the timeseries schema snapshot can still be generated
+ * and sent to the followers.
+ *
+ * @throws IOException
+ */
+ private void collectTsFilesAndFillTimeseriesSchemas() throws IOException {
+ // 1.collect tsfile
+ collectTsFiles();
+
+ //2.register the measurement
+ for (Map.Entry<Integer, Collection<TimeseriesSchema>> entry : slotTimeseries.entrySet()) {
+ int slotNum = entry.getKey();
+ FileSnapshot snapshot = slotSnapshots.computeIfAbsent(slotNum,
+ s -> new FileSnapshot());
+ if (snapshot.getTimeseriesSchemas().isEmpty()) {
+ snapshot.setTimeseriesSchemas(slotTimeseries.getOrDefault(slotNum,
+ Collections.emptySet()));
Review comment:
Why not just use `entry.value()`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-iotdb] jt2594838 merged pull request #1560: [Distributed] bug fix: Leader cannot be found
Posted by GitBox <gi...@apache.org>.
jt2594838 merged pull request #1560:
URL: https://github.com/apache/incubator-iotdb/pull/1560
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org