LebronAl commented on a change in pull request #3191:
URL: https://github.com/apache/iotdb/pull/3191#discussion_r643614637
##########
File path:
cluster/src/main/java/org/apache/iotdb/cluster/coordinator/Coordinator.java
##########
@@ -442,9 +445,11 @@ private TSStatus forwardToMultipleGroup(Map<PhysicalPlan,
PartitionGroup> planGr
}
TSStatus status;
if (errorCodePartitionGroups.isEmpty()) {
- status = StatusUtils.OK;
if (allRedirect) {
- status = StatusUtils.getStatus(status, endPoint);
+ status = new TSStatus(TSStatusCode.NEED_REDIRECTION.getStatusCode());
Review comment:
maybe these two lines can be wrapped into a function in `StatusUtils`
##########
File path:
cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -890,4 +988,57 @@ public boolean isUnchanged() {
public void setUnchanged(boolean unchanged) {
this.unchanged = unchanged;
}
+
+ public void setAndSaveLastAppliedPartitionTableVersion(long version) {
+ lastAppliedPartitionTableVersion.setVersion(version);
+ lastAppliedPartitionTableVersion.save();
+ }
+
+ private class LastAppliedPatitionTableVersion {
+
+ private static final String VERSION_FILE_NAME =
"LAST_PARTITION_TABLE_VERSION";
+
+ private long version = -1;
+
+ private String filePath;
+
+ public LastAppliedPatitionTableVersion(String memberDir) {
+ this.filePath = memberDir + File.separator + VERSION_FILE_NAME;
+ load();
+ }
+
+ private void load() {
+ File versionFile = new File(filePath);
+ if (!versionFile.exists()) {
+ return;
+ }
+ try (FileInputStream fileInputStream = new FileInputStream(filePath);
+ DataInputStream dataInputStream = new
DataInputStream(fileInputStream)) {
+ version = dataInputStream.readLong();
+ } catch (Exception e) {
+ logger.warn("Cannot deserialize last partition table version from {}",
filePath, e);
+ }
+ }
+
+ public synchronized void save() {
Review comment:
this should be atomic? maybe we need a atomic-rename mechanism
##########
File path:
cluster/src/main/java/org/apache/iotdb/cluster/client/sync/SyncClientAdaptor.java
##########
@@ -165,14 +165,14 @@ public static Long querySingleSeries(
}
public static List<String> getNodeList(
Review comment:
Why `querySingleSeriesByTimestamp` and `querySingleSeries` function
don't have `raftId`? It seems that these query function's behavior should be
consistent?
##########
File path:
cluster/src/main/java/org/apache/iotdb/cluster/client/sync/SyncClientAdaptor.java
##########
@@ -106,13 +106,13 @@ public static Long removeNode(AsyncMetaClient
asyncMetaClient, Node nodeToRemove
}
public static Boolean matchTerm(
- AsyncClient client, Node target, long prevLogIndex, long prevLogTerm,
Node header)
Review comment:
As there already has a RaftNode struct in `cluster.thrift`, maybe we can
change these rpc parameters from `Node header,int raftId` to `RaftNode
raftNode`, this will be much clearer
##########
File path:
cluster/src/main/java/org/apache/iotdb/cluster/client/sync/SyncClientAdaptor.java
##########
@@ -435,32 +440,42 @@ public static TSStatus executeNonQuery(
}
public static ByteBuffer readFile(
- AsyncDataClient client, String remotePath, long offset, int fetchSize)
+ AsyncDataClient client, String remotePath, long offset, int fetchSize,
int raftId)
Review comment:
Maybe `raftId` is not needed in `readFile` rpc and some other rpcs?
--
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:
[email protected]
