Skip to content

Commit 1e8eee6

Browse files
authored
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
1 parent 892ffe2 commit 1e8eee6

5 files changed

Lines changed: 88 additions & 4 deletions

File tree

storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ public BlobStoreFileInputStream(BlobStoreFile part) throws IOException {
444444

445445
@Override
446446
public long getVersion() throws IOException {
447-
return part.getModTime();
447+
return part.getVersion();
448448
}
449449

450450
@Override

storm-client/src/jvm/org/apache/storm/blobstore/BlobStoreFile.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ public abstract class BlobStoreFile {
4242

4343
public abstract long getModTime() throws IOException;
4444

45+
public long getVersion() throws IOException {
46+
return getModTime();
47+
}
48+
4549
public abstract InputStream getInputStream() throws IOException;
4650

4751
public abstract OutputStream getOutputStream() throws IOException;

storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ public ReadableBlobMeta getBlobMeta(String key, Subject who) throws Authorizatio
291291
rbm.set_settable(meta);
292292
try {
293293
LocalFsBlobStoreFile pf = fbs.read(DATA_PREFIX + key);
294-
rbm.set_version(pf.getModTime());
294+
rbm.set_version(pf.getVersion());
295295
} catch (IOException e) {
296296
throw new RuntimeException(e);
297297
}

storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStoreFile.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
* Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with
33
* this work for additional information regarding copyright ownership. The ASF licenses this file to you under the Apache License, Version
44
* 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
5-
*
5+
* <p>
66
* http://www.apache.org/licenses/LICENSE-2.0
7-
*
7+
* <p>
88
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS,
99
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
1010
* and limitations under the License.
@@ -21,14 +21,20 @@
2121
import java.nio.file.Files;
2222
import java.nio.file.StandardCopyOption;
2323
import java.util.regex.Matcher;
24+
import java.util.zip.CRC32C;
25+
import java.util.zip.Checksum;
26+
27+
import org.apache.commons.io.FileUtils;
2428
import org.apache.storm.generated.SettableBlobMeta;
2529

30+
2631
public class LocalFsBlobStoreFile extends BlobStoreFile {
2732

2833
private final String key;
2934
private final boolean isTmp;
3035
private final File path;
3136
private final boolean mustBeNew;
37+
private final Checksum checksumAlgorithm;
3238
private SettableBlobMeta meta;
3339

3440
public LocalFsBlobStoreFile(File base, String name) {
@@ -44,12 +50,14 @@ public LocalFsBlobStoreFile(File base, String name) {
4450
key = base.getName();
4551
path = new File(base, name);
4652
mustBeNew = false;
53+
checksumAlgorithm = new CRC32C();
4754
}
4855

4956
public LocalFsBlobStoreFile(File base, boolean isTmp, boolean mustBeNew) {
5057
key = base.getName();
5158
this.isTmp = isTmp;
5259
this.mustBeNew = mustBeNew;
60+
checksumAlgorithm = new CRC32C();
5361
if (this.isTmp) {
5462
path = new File(base, System.currentTimeMillis() + TMP_EXT);
5563
} else {
@@ -72,6 +80,11 @@ public String getKey() {
7280
return key;
7381
}
7482

83+
@Override
84+
public long getVersion() throws IOException {
85+
return FileUtils.checksum(path, checksumAlgorithm).getValue();
86+
}
87+
7588
@Override
7689
public long getModTime() throws IOException {
7790
return path.lastModified();
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with
3+
* this work for additional information regarding copyright ownership. The ASF licenses this file to you under the Apache License, Version
4+
* 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
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
10+
* and limitations under the License.
11+
*/
12+
13+
package org.apache.storm.blobstore;
14+
15+
import org.apache.commons.io.FileUtils;
16+
import org.junit.jupiter.api.BeforeEach;
17+
import org.junit.jupiter.api.Test;
18+
19+
import java.io.File;
20+
import java.io.FileOutputStream;
21+
import java.io.IOException;
22+
import java.nio.file.Files;
23+
import java.util.zip.CRC32C;
24+
25+
import static org.junit.jupiter.api.Assertions.assertEquals;
26+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
27+
28+
class LocalFsBlobStoreFileTest {
29+
30+
private File tempFile;
31+
private LocalFsBlobStoreFile blobStoreFile;
32+
private CRC32C checksumAlgorithm;
33+
34+
@BeforeEach
35+
public void setUp() throws IOException {
36+
tempFile = Files.createTempFile(null, ".tmp").toFile();
37+
try (FileOutputStream fs = new FileOutputStream(tempFile)) {
38+
fs.write("Content for checksum".getBytes());
39+
}
40+
blobStoreFile = new LocalFsBlobStoreFile(tempFile.getParentFile(), tempFile.getName());
41+
checksumAlgorithm= new CRC32C();
42+
}
43+
44+
@Test
45+
void testGetVersion() throws IOException {
46+
long expectedVersion = FileUtils.checksum(tempFile, checksumAlgorithm).getValue();
47+
long actualVersion = blobStoreFile.getVersion();
48+
assertEquals(expectedVersion, actualVersion, "The version should match the expected checksum value.");
49+
}
50+
51+
@Test
52+
void testGetVersion_Mismatch() throws IOException {
53+
long expectedVersion = FileUtils.checksum(tempFile, checksumAlgorithm).getValue();
54+
try (FileOutputStream fs = new FileOutputStream(tempFile)) {
55+
fs.write("Different content".getBytes());
56+
}
57+
long actualVersion = blobStoreFile.getVersion();
58+
assertNotEquals(expectedVersion, actualVersion, "The version shouldn't match the checksum value of different content.");
59+
}
60+
61+
@Test
62+
void testGetModTime() throws IOException {
63+
long expectedModTime = tempFile.lastModified();
64+
long actualModTime = blobStoreFile.getModTime();
65+
assertEquals(expectedModTime, actualModTime, "The modification time should match the expected value.");
66+
}
67+
}

0 commit comments

Comments
 (0)