This is an automated email from the ASF dual-hosted git repository.
rabreu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git
The following commit(s) were added to refs/heads/master by this push:
new 1e8eee6bd Use SHA for BLOB update instead of modification time (#3697)
1e8eee6bd is described below
commit 1e8eee6bde9d96d2e177a272bc06a1af59b0a6ca
Author: paxadax <[email protected]>
AuthorDate: Fri Oct 4 12:14:51 2024 +0100
Use SHA for BLOB update instead of modification time (#3697)
* feat: Use file SHA instead of last modification time
* tests: Add unit tests for SHA version
* fix: add missing comment for rat-plugin
* fix: Use sha256 instead of sha1
* fix: fix tests
* feat: Use Checksum instead of hash for faster computation
* tests: Add tests for checksum
---
.../jvm/org/apache/storm/blobstore/BlobStore.java | 2 +-
.../org/apache/storm/blobstore/BlobStoreFile.java | 4 ++
.../apache/storm/blobstore/LocalFsBlobStore.java | 2 +-
.../storm/blobstore/LocalFsBlobStoreFile.java | 17 +++++-
.../storm/blobstore/LocalFsBlobStoreFileTest.java | 67 ++++++++++++++++++++++
5 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
b/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
index 8323884ae..367141ae5 100644
--- a/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
+++ b/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
@@ -444,7 +444,7 @@ public abstract class BlobStore implements Shutdownable,
AutoCloseable {
@Override
public long getVersion() throws IOException {
- return part.getModTime();
+ return part.getVersion();
}
@Override
diff --git a/storm-client/src/jvm/org/apache/storm/blobstore/BlobStoreFile.java
b/storm-client/src/jvm/org/apache/storm/blobstore/BlobStoreFile.java
index 2f47978b3..bd901a0c2 100644
--- a/storm-client/src/jvm/org/apache/storm/blobstore/BlobStoreFile.java
+++ b/storm-client/src/jvm/org/apache/storm/blobstore/BlobStoreFile.java
@@ -42,6 +42,10 @@ public abstract class BlobStoreFile {
public abstract long getModTime() throws IOException;
+ public long getVersion() throws IOException {
+ return getModTime();
+ }
+
public abstract InputStream getInputStream() throws IOException;
public abstract OutputStream getOutputStream() throws IOException;
diff --git
a/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java
b/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java
index a8f519d64..f708946fb 100644
---
a/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java
+++
b/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java
@@ -291,7 +291,7 @@ public class LocalFsBlobStore extends BlobStore {
rbm.set_settable(meta);
try {
LocalFsBlobStoreFile pf = fbs.read(DATA_PREFIX + key);
- rbm.set_version(pf.getModTime());
+ rbm.set_version(pf.getVersion());
} catch (IOException e) {
throw new RuntimeException(e);
}
diff --git
a/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStoreFile.java
b/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStoreFile.java
index 2262e9081..128377f98 100644
---
a/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStoreFile.java
+++
b/storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStoreFile.java
@@ -2,9 +2,9 @@
* 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
- *
+ * <p>
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ * <p>
* 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.
@@ -21,14 +21,20 @@ import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
import java.util.regex.Matcher;
+import java.util.zip.CRC32C;
+import java.util.zip.Checksum;
+
+import org.apache.commons.io.FileUtils;
import org.apache.storm.generated.SettableBlobMeta;
+
public class LocalFsBlobStoreFile extends BlobStoreFile {
private final String key;
private final boolean isTmp;
private final File path;
private final boolean mustBeNew;
+ private final Checksum checksumAlgorithm;
private SettableBlobMeta meta;
public LocalFsBlobStoreFile(File base, String name) {
@@ -44,12 +50,14 @@ public class LocalFsBlobStoreFile extends BlobStoreFile {
key = base.getName();
path = new File(base, name);
mustBeNew = false;
+ checksumAlgorithm = new CRC32C();
}
public LocalFsBlobStoreFile(File base, boolean isTmp, boolean mustBeNew) {
key = base.getName();
this.isTmp = isTmp;
this.mustBeNew = mustBeNew;
+ checksumAlgorithm = new CRC32C();
if (this.isTmp) {
path = new File(base, System.currentTimeMillis() + TMP_EXT);
} else {
@@ -72,6 +80,11 @@ public class LocalFsBlobStoreFile extends BlobStoreFile {
return key;
}
+ @Override
+ public long getVersion() throws IOException {
+ return FileUtils.checksum(path, checksumAlgorithm).getValue();
+ }
+
@Override
public long getModTime() throws IOException {
return path.lastModified();
diff --git
a/storm-server/src/test/java/org/apache/storm/blobstore/LocalFsBlobStoreFileTest.java
b/storm-server/src/test/java/org/apache/storm/blobstore/LocalFsBlobStoreFileTest.java
new file mode 100644
index 000000000..2faae2a51
--- /dev/null
+++
b/storm-server/src/test/java/org/apache/storm/blobstore/LocalFsBlobStoreFileTest.java
@@ -0,0 +1,67 @@
+/**
+ * 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.storm.blobstore;
+
+import org.apache.commons.io.FileUtils;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.zip.CRC32C;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+class LocalFsBlobStoreFileTest {
+
+ private File tempFile;
+ private LocalFsBlobStoreFile blobStoreFile;
+ private CRC32C checksumAlgorithm;
+
+ @BeforeEach
+ public void setUp() throws IOException {
+ tempFile = Files.createTempFile(null, ".tmp").toFile();
+ try (FileOutputStream fs = new FileOutputStream(tempFile)) {
+ fs.write("Content for checksum".getBytes());
+ }
+ blobStoreFile = new LocalFsBlobStoreFile(tempFile.getParentFile(),
tempFile.getName());
+ checksumAlgorithm= new CRC32C();
+ }
+
+ @Test
+ void testGetVersion() throws IOException {
+ long expectedVersion = FileUtils.checksum(tempFile,
checksumAlgorithm).getValue();
+ long actualVersion = blobStoreFile.getVersion();
+ assertEquals(expectedVersion, actualVersion, "The version should match
the expected checksum value.");
+ }
+
+ @Test
+ void testGetVersion_Mismatch() throws IOException {
+ long expectedVersion = FileUtils.checksum(tempFile,
checksumAlgorithm).getValue();
+ try (FileOutputStream fs = new FileOutputStream(tempFile)) {
+ fs.write("Different content".getBytes());
+ }
+ long actualVersion = blobStoreFile.getVersion();
+ assertNotEquals(expectedVersion, actualVersion, "The version shouldn't
match the checksum value of different content.");
+ }
+
+ @Test
+ void testGetModTime() throws IOException {
+ long expectedModTime = tempFile.lastModified();
+ long actualModTime = blobStoreFile.getModTime();
+ assertEquals(expectedModTime, actualModTime, "The modification time
should match the expected value.");
+ }
+}