You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2022/07/27 00:29:57 UTC
[doris] branch dev-1.1.2 updated: [fix](image) fix bug that latestValidatedImageSeq may not be the second largest image id (#11201) (#11230)
This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch dev-1.1.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/dev-1.1.2 by this push:
new 9335a46340 [fix](image) fix bug that latestValidatedImageSeq may not be the second largest image id (#11201) (#11230)
9335a46340 is described below
commit 9335a46340fb599bd1f631f7d514d970ebcb0f57
Author: yiguolei <67...@qq.com>
AuthorDate: Wed Jul 27 08:29:53 2022 +0800
[fix](image) fix bug that latestValidatedImageSeq may not be the second largest image id (#11201) (#11230)
* [fix](image) fix bug that latestValidatedImageSeq may not be the second largest image id
When traversing the image files in the meta directory,
it cannot be guaranteed to be traversed in the order of imageid size
For example, if it traverses the image file with orders like: 3,5,4,1,
then latestImageSeq is 5, but latestValidatedImageSeq is 3, which is wrong.
Co-authored-by: Mingyu Chen <mo...@gmail.com>
---
.github/PULL_REQUEST_TEMPLATE.md | 34 ++++++++++++----
.../java/org/apache/doris/persist/Storage.java | 29 +++++--------
.../java/org/apache/doris/persist/StorageTest.java | 47 +++++++++-------------
3 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index 6f56b55bac..7e4ae879df 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -4,16 +4,36 @@ Issue Number: close #xxx
## Problem Summary:
-Describe the overview of changes.
-
## Checklist(Required)
-1. Does it affect the original behavior: (Yes/No/I Don't know)
-2. Has unit tests been added: (Yes/No/No Need)
-3. Has document been added or modified: (Yes/No/No Need)
-4. Does it need to update dependencies: (Yes/No)
-5. Are there any changes that cannot be rolled back: (Yes/No)
+1. Type of your changes:
+ - [ ] Improvement
+ - [ ] Fix
+ - [ ] Feature-WIP
+ - [ ] Feature
+ - [ ] Doc
+ - [ ] Refator
+ - [ ] Others:
+2. Does it affect the original behavior:
+ - [ ] Yes
+ - [ ] No
+ - [ ] I don't know
+3. Has unit tests been added:
+ - [ ] Yes
+ - [ ] No
+ - [ ] No Need
+4. Has document been added or modified:
+ - [ ] Yes
+ - [ ] No
+ - [ ] No Need
+5. Does it need to update dependencies:
+ - [ ] Yes
+ - [ ] No
+6. Are there any changes that cannot be rolled back:
+ - [ ] Yes
+ - [ ] No
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
+
diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/Storage.java b/fe/fe-core/src/main/java/org/apache/doris/persist/Storage.java
index 8eb0336d71..3f2f5bd507 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/Storage.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/Storage.java
@@ -21,7 +21,7 @@ import org.apache.doris.ha.FrontendNodeType;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
-
+import com.google.common.collect.Lists;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -60,8 +60,8 @@ public class Storage {
private FrontendNodeType role = FrontendNodeType.UNKNOWN;
private String nodeName;
private long editsSeq;
- private long latestImageSeq;
- private long latestValidatedImageSeq;
+ private long latestImageSeq = 0;
+ private long latestValidatedImageSeq = 0;
private String metaDir;
private List<Long> editsFileSequenceNumbers;
@@ -121,6 +121,7 @@ public class Storage {
if (children == null) {
return;
}
+ List<Long> imageIds = Lists.newArrayList();
for (File child : children) {
String name = child.getName();
try {
@@ -128,8 +129,8 @@ public class Storage {
&& !name.endsWith(".part") && name.contains(".")) {
if (name.startsWith(IMAGE)) {
long fileSeq = Long.parseLong(name.substring(name.lastIndexOf('.') + 1));
+ imageIds.add(fileSeq);
if (latestImageSeq < fileSeq) {
- latestValidatedImageSeq = latestImageSeq;
latestImageSeq = fileSeq;
}
} else if (name.startsWith(EDITS)) {
@@ -142,16 +143,15 @@ public class Storage {
LOG.warn(name + " is not a validate meta file, ignore it");
}
}
+ // set latestValidatedImageSeq to the second largest image id, or 0 if less than 2 images.
+ Collections.sort(imageIds);
+ latestValidatedImageSeq = imageIds.size() < 2 ? 0 : imageIds.get(imageIds.size() - 2);
}
public int getClusterID() {
return clusterID;
}
- public void setClusterID(int clusterID) {
- this.clusterID = clusterID;
- }
-
public String getToken() {
return token;
}
@@ -172,10 +172,6 @@ public class Storage {
return metaDir;
}
- public void setMetaDir(String metaDir) {
- this.metaDir = metaDir;
- }
-
public long getLatestImageSeq() {
return latestImageSeq;
}
@@ -184,14 +180,6 @@ public class Storage {
return latestValidatedImageSeq;
}
- public void setLatestImageSeq(long latestImageSeq) {
- this.latestImageSeq = latestImageSeq;
- }
-
- public void setEditsSeq(long editsSeq) {
- this.editsSeq = editsSeq;
- }
-
public long getEditsSeq() {
return editsSeq;
}
@@ -253,6 +241,7 @@ public class Storage {
}
}
+ // Only for test
public void clear() throws IOException {
File metaFile = new File(metaDir);
if (metaFile.exists()) {
diff --git a/fe/fe-core/src/test/java/org/apache/doris/persist/StorageTest.java b/fe/fe-core/src/test/java/org/apache/doris/persist/StorageTest.java
index ca17a0006c..2610e0b8e3 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/persist/StorageTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/persist/StorageTest.java
@@ -26,6 +26,7 @@ import java.io.IOException;
public class StorageTest {
private String meta = "storageTestDir/";
+ private static long clusterId = 966271669;
public void mkdir() {
File dir = new File(meta);
@@ -42,11 +43,13 @@ public class StorageTest {
}
public void addFiles(int image, int edit) {
- File imageFile = new File(meta + "image." + image);
- try {
- imageFile.createNewFile();
- } catch (IOException e) {
- e.printStackTrace();
+ for (int i = 1; i <= image; i++) {
+ File imageFile = new File(meta + "image." + i);
+ try {
+ imageFile.createNewFile();
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
}
for (int i = 1; i <= edit; i++) {
@@ -69,7 +72,7 @@ public class StorageTest {
try {
version.createNewFile();
String line1 = "#Mon Feb 02 13:59:54 CST 2015\n";
- String line2 = "clusterId=966271669";
+ String line2 = "clusterId=" + clusterId;
FileWriter fw = new FileWriter(version);
fw.write(line1);
fw.write(line2);
@@ -89,7 +92,6 @@ public class StorageTest {
file.delete();
}
}
-
dir.delete();
}
}
@@ -110,40 +112,29 @@ public class StorageTest {
@Test
public void testStorage() throws Exception {
mkdir();
- addFiles(0, 10);
-
+ addFiles(5, 10);
Storage storage = new Storage("storageTestDir");
Assert.assertEquals(966271669, storage.getClusterID());
- storage.setClusterID(1234);
- Assert.assertEquals(1234, storage.getClusterID());
- Assert.assertEquals(0, storage.getLatestImageSeq());
+ Assert.assertEquals(5, storage.getLatestImageSeq());
+ Assert.assertEquals(4, storage.getLatestValidatedImageSeq());
Assert.assertEquals(10, Storage.getMetaSeq(new File("storageTestDir/edits.10")));
- Assert.assertTrue(Storage.getCurrentEditsFile(new File("storageTestDir"))
- .equals(new File("storageTestDir/edits")));
+ Assert.assertTrue(
+ Storage.getCurrentEditsFile(new File("storageTestDir")).equals(new File("storageTestDir/edits")));
- Assert.assertTrue(storage.getCurrentImageFile().equals(new File("storageTestDir/image.0")));
+ Assert.assertTrue(storage.getCurrentImageFile().equals(new File("storageTestDir/image.5")));
Assert.assertTrue(storage.getImageFile(0).equals(new File("storageTestDir/image.0")));
- Assert.assertTrue(Storage.getImageFile(new File("storageTestDir"), 0)
- .equals(new File("storageTestDir/image.0")));
+ Assert.assertTrue(
+ Storage.getImageFile(new File("storageTestDir"), 0).equals(new File("storageTestDir/image.0")));
Assert.assertTrue(storage.getCurrentEditsFile().equals(new File("storageTestDir/edits")));
Assert.assertTrue(storage.getEditsFile(5).equals(new File("storageTestDir/edits.5")));
- Assert.assertTrue(Storage.getEditsFile(new File("storageTestDir"), 3)
- .equals(new File("storageTestDir/edits.3")));
+ Assert.assertTrue(
+ Storage.getEditsFile(new File("storageTestDir"), 3).equals(new File("storageTestDir/edits.3")));
Assert.assertTrue(storage.getVersionFile().equals(new File("storageTestDir/VERSION")));
- storage.setLatestImageSeq(100);
- Assert.assertEquals(100, storage.getLatestImageSeq());
-
- storage.setEditsSeq(100);
- Assert.assertEquals(100, storage.getEditsSeq());
-
Assert.assertEquals("storageTestDir", storage.getMetaDir());
- storage.setMetaDir("abcd");
- Assert.assertEquals("abcd", storage.getMetaDir());
- storage.setMetaDir("storageTestDir");
storage.clear();
File file = new File(storage.getMetaDir());
Assert.assertEquals(0, file.list().length);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org