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/04/27 02:11:11 UTC
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #1094: [IOTDB-613] Add the ability to persist and maintain currentterm for the raft node
jt2594838 commented on a change in pull request #1094:
URL: https://github.com/apache/incubator-iotdb/pull/1094#discussion_r415460235
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/manage/UnCommittedEntryManager.java
##########
@@ -126,11 +128,11 @@ public void truncateAndAppend(List<Log> appendingEntries) {
long len = after - offset;
if (len < 0) {
// the logs are being truncated to before our current offset portion, which is committed entries
- // unconditional obedience to the leader's request. Maybe throw a exception here is better
- offset = after;
- entries = appendingEntries;
+ // Maybe throw a exception here is better
logger.error("The logs which first index is {} are going to truncate committed logs",
after);
+// throw new TruncateCommittedEntryException(appendingEntries.get(0).getCurrLogIndex(),
+// offset);
Review comment:
Please just decide one and do not leave commented code. Both are okay for me.
##########
File path: cluster/src/test/java/org/apache/iotdb/cluster/log/manage/RaftLogManagerTest.java
##########
@@ -48,12 +52,22 @@ public void apply(Log log) {
appliedLogs.add(log);
}
};
+ private int testIdentifier = 1;
@Before
public void setUp() {
appliedLogs = new HashSet<>();
}
+ @After
+ public void tearDown() {
+ File dir = new File(new SyncLogDequeSerializer(testIdentifier).getLogDir());
+ for (File file : dir.listFiles()) {
+ file.delete();
+ }
+ dir.delete();
+ }
Review comment:
Please check the indents.
##########
File path: cluster/src/test/java/org/apache/iotdb/cluster/log/manage/serializable/SyncLogDequeSerializerTest.java
##########
@@ -354,99 +222,87 @@ public void testRemoveOldFileAtRecovery() {
}
}
-
- @Test
- public void testTruncate() {
- SyncLogDequeSerializer syncLogDequeSerializer = new SyncLogDequeSerializer(getNode(1));
- List<Log> testLogs2;
- try {
- syncLogDequeSerializer.setMaxRemovedLogSize(10);
-
- MemoryLogManager memoryLogManager = buildMemoryLogManager();
- List<Log> testLogs1 = TestUtils.prepareNodeLogs(10);
-
- for (Log log : testLogs1) {
- memoryLogManager.appendLog(log);
- syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
- }
-
- assertEquals(10, syncLogDequeSerializer.getLogSizeDeque().size());
-
- syncLogDequeSerializer.removeFirst(3);
-
- testLogs2 = TestUtils.prepareNodeLogs(10);
-
- for (Log log : testLogs2) {
- memoryLogManager.appendLog(log);
- syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
- }
-
- assertEquals(17, syncLogDequeSerializer.getLogSizeDeque().size());
-
- assertEquals(2, syncLogDequeSerializer.logFileList.size());
- // this will remove first file and build a new file
- syncLogDequeSerializer.removeFirst(8);
-
- assertEquals(9, syncLogDequeSerializer.getLogSizeDeque().size());
- assertEquals(2, syncLogDequeSerializer.logFileList.size());
-
- List<Log> testLogs3 = TestUtils.prepareNodeLogs(10);
- for (Log log : testLogs3) {
- memoryLogManager.appendLog(log);
- syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
- }
- assertEquals(19, syncLogDequeSerializer.getLogSizeDeque().size());
- syncLogDequeSerializer.truncateLog(11, memoryLogManager.getMeta());
-
- // last file has been truncated
- assertEquals(1, syncLogDequeSerializer.logFileList.size());
- } finally {
- syncLogDequeSerializer.close();
- }
-
- // recovery
- syncLogDequeSerializer = new SyncLogDequeSerializer(getNode(1));
- try {
- List<Log> logs = syncLogDequeSerializer.recoverLog();
- assertEquals(9, logs.size());
-
- for (int i = 0; i < 8; i++) {
- assertEquals(testLogs2.get(i + 1), logs.get(i));
- }
- } finally {
- syncLogDequeSerializer.close();
- }
- }
+// @Test
+// public void testTruncate() {
+// SyncLogDequeSerializer syncLogDequeSerializer = new SyncLogDequeSerializer(testIdentifier);
+// List<Log> testLogs2;
+// try {
+// syncLogDequeSerializer.setMaxRemovedLogSize(10);
+//
+// MemoryLogManager memoryLogManager = buildMemoryLogManager();
+// List<Log> testLogs1 = TestUtils.prepareNodeLogs(10);
+//
+// for (Log log : testLogs1) {
+// memoryLogManager.appendLog(log);
+// syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
+// }
+//
+// assertEquals(10, syncLogDequeSerializer.getLogSizeDeque().size());
+//
+// syncLogDequeSerializer.removeFirst(3);
+//
+// testLogs2 = TestUtils.prepareNodeLogs(10);
+//
+// for (Log log : testLogs2) {
+// memoryLogManager.appendLog(log);
+// syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
+// }
+//
+// assertEquals(17, syncLogDequeSerializer.getLogSizeDeque().size());
+//
+// assertEquals(2, syncLogDequeSerializer.logFileList.size());
+// // this will remove first file and build a new file
+// syncLogDequeSerializer.removeFirst(8);
+//
+// assertEquals(9, syncLogDequeSerializer.getLogSizeDeque().size());
+// assertEquals(2, syncLogDequeSerializer.logFileList.size());
+//
+// List<Log> testLogs3 = TestUtils.prepareNodeLogs(10);
+// for (Log log : testLogs3) {
+// memoryLogManager.appendLog(log);
+// syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
+// }
+// assertEquals(19, syncLogDequeSerializer.getLogSizeDeque().size());
+// syncLogDequeSerializer.truncateLog(11, memoryLogManager.getMeta());
+//
+// // last file has been truncated
+// assertEquals(1, syncLogDequeSerializer.logFileList.size());
+// } finally {
+// syncLogDequeSerializer.close();
+// }
+//
+// // recovery
+// syncLogDequeSerializer = new SyncLogDequeSerializer(testIdentifier);
+// try {
+// List<Log> logs = syncLogDequeSerializer.recoverLog();
+// assertEquals(9, logs.size());
+//
+// for (int i = 0; i < 8; i++) {
+// assertEquals(testLogs2.get(i + 1), logs.get(i));
+// }
+// } finally {
+// syncLogDequeSerializer.close();
+// }
+// }
Review comment:
Remove these if they are not necessary.
##########
File path: cluster/src/test/java/org/apache/iotdb/cluster/log/manage/serializable/SyncLogDequeSerializerTest.java
##########
@@ -458,85 +314,78 @@ public void testRecoveryByAppendList() {
}
}
- @Test
- public void testTruncateByAppendList() {
- SyncLogDequeSerializer syncLogDequeSerializer = new SyncLogDequeSerializer(getNode(1));
- List<Log> testLogs2;
- try {
- syncLogDequeSerializer.setMaxRemovedLogSize(10);
-
- MemoryLogManager memoryLogManager = buildMemoryLogManager();
- List<Log> testLogs1 = TestUtils.prepareNodeLogs(10);
-
- for (Log log : testLogs1) {
- memoryLogManager.appendLog(log);
- }
- syncLogDequeSerializer.append(testLogs1, memoryLogManager.getMeta());
-
- assertEquals(10, syncLogDequeSerializer.getLogSizeDeque().size());
-
- syncLogDequeSerializer.removeFirst(3);
-
- testLogs2 = TestUtils.prepareNodeLogs(10);
-
- for (Log log : testLogs2) {
- memoryLogManager.appendLog(log);
- }
- syncLogDequeSerializer.append(testLogs2, memoryLogManager.getMeta());
-
- assertEquals(17, syncLogDequeSerializer.getLogSizeDeque().size());
-
- assertEquals(2, syncLogDequeSerializer.logFileList.size());
- // this will remove first file and build a new file
- syncLogDequeSerializer.removeFirst(8);
-
- assertEquals(9, syncLogDequeSerializer.getLogSizeDeque().size());
- assertEquals(2, syncLogDequeSerializer.logFileList.size());
-
- List<Log> testLogs3 = TestUtils.prepareNodeLogs(10);
- for (Log log : testLogs3) {
- memoryLogManager.appendLog(log);
- syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
- }
- assertEquals(19, syncLogDequeSerializer.getLogSizeDeque().size());
- syncLogDequeSerializer.truncateLog(11, memoryLogManager.getMeta());
-
- // last file has been truncated
- assertEquals(1, syncLogDequeSerializer.logFileList.size());
- } finally {
- syncLogDequeSerializer.close();
- }
-
- // recovery
- syncLogDequeSerializer = new SyncLogDequeSerializer(getNode(1));
- try {
- List<Log> logs = syncLogDequeSerializer.recoverLog();
- assertEquals(9, logs.size());
-
- for (int i = 0; i < 8; i++) {
- assertEquals(testLogs2.get(i + 1), logs.get(i));
- }
- } finally {
- syncLogDequeSerializer.close();
- }
- }
+// @Test
+// public void testTruncateByAppendList() {
+// SyncLogDequeSerializer syncLogDequeSerializer = new SyncLogDequeSerializer(testIdentifier);
+// List<Log> testLogs2;
+// try {
+// syncLogDequeSerializer.setMaxRemovedLogSize(10);
+//
+// MemoryLogManager memoryLogManager = buildMemoryLogManager();
+// List<Log> testLogs1 = TestUtils.prepareNodeLogs(10);
+//
+// for (Log log : testLogs1) {
+// memoryLogManager.appendLog(log);
+// }
+// syncLogDequeSerializer.append(testLogs1, memoryLogManager.getMeta());
+//
+// assertEquals(10, syncLogDequeSerializer.getLogSizeDeque().size());
+//
+// syncLogDequeSerializer.removeFirst(3);
+//
+// testLogs2 = TestUtils.prepareNodeLogs(10);
+//
+// for (Log log : testLogs2) {
+// memoryLogManager.appendLog(log);
+// }
+// syncLogDequeSerializer.append(testLogs2, memoryLogManager.getMeta());
+//
+// assertEquals(17, syncLogDequeSerializer.getLogSizeDeque().size());
+//
+// assertEquals(2, syncLogDequeSerializer.logFileList.size());
+// // this will remove first file and build a new file
+// syncLogDequeSerializer.removeFirst(8);
+//
+// assertEquals(9, syncLogDequeSerializer.getLogSizeDeque().size());
+// assertEquals(2, syncLogDequeSerializer.logFileList.size());
+//
+// List<Log> testLogs3 = TestUtils.prepareNodeLogs(10);
+// for (Log log : testLogs3) {
+// memoryLogManager.appendLog(log);
+// syncLogDequeSerializer.append(log, memoryLogManager.getMeta());
+// }
+// assertEquals(19, syncLogDequeSerializer.getLogSizeDeque().size());
+// syncLogDequeSerializer.truncateLog(11, memoryLogManager.getMeta());
+//
+// // last file has been truncated
+// assertEquals(1, syncLogDequeSerializer.logFileList.size());
+// } finally {
+// syncLogDequeSerializer.close();
+// }
+//
+// // recovery
+// syncLogDequeSerializer = new SyncLogDequeSerializer(testIdentifier);
+// try {
+// List<Log> logs = syncLogDequeSerializer.recoverLog();
+// assertEquals(9, logs.size());
+//
+// for (int i = 0; i < 8; i++) {
+// assertEquals(testLogs2.get(i + 1), logs.get(i));
+// }
+// } finally {
+// syncLogDequeSerializer.close();
+// }
+// }
Review comment:
Remove these if they are not necessary.
----------------------------------------------------------------
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