Skip to content

Commit 5ff8d5b

Browse files
author
Anuj Modi
committed
Resolved Comments
1 parent 1f03c73 commit 5ff8d5b

File tree

6 files changed

+24
-11
lines changed

6 files changed

+24
-11
lines changed

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1159,7 +1159,8 @@ public boolean getIsChecksumValidationEnabled() {
11591159
return isChecksumValidationEnabled;
11601160
}
11611161

1162-
void setIsChecksumValidationEnabled(boolean isChecksumEnabled) {
1162+
@VisibleForTesting
1163+
public void setIsChecksumValidationEnabled(boolean isChecksumEnabled) {
11631164
this.isChecksumValidationEnabled = isChecksumEnabled;
11641165
}
11651166
}

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.concurrent.TimeUnit;
4040

4141
import org.apache.hadoop.classification.VisibleForTesting;
42+
import org.apache.hadoop.fs.PathIOException;
4243
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidChecksumException;
4344
import org.apache.hadoop.fs.store.LogExactlyOnce;
4445
import org.apache.hadoop.util.Preconditions;
@@ -1012,7 +1013,7 @@ public AbfsRestOperation read(final String path, final long position, final byte
10121013
op.execute(tracingContext);
10131014

10141015
if (isChecksumValidationEnabled(requestHeaders, rangeHeader, bufferLength)) {
1015-
verifyCheckSumForRead(buffer, op.getResult(), bufferOffset);
1016+
verifyCheckSumForRead(buffer, path, op.getResult(), bufferOffset);
10161017
}
10171018

10181019
return op;
@@ -1451,11 +1452,13 @@ private void addCheckSumHeaderForWrite(List<AbfsHttpHeader> requestHeaders,
14511452
/**
14521453
* To verify the checksum information received from server for the data read
14531454
* @param buffer stores the data received from server
1455+
* @param pathStr stores the path being read
14541456
* @param result HTTP Operation Result
14551457
* @param bufferOffset Position where data returned by server is saved in buffer
14561458
* @throws AbfsRestOperationException
14571459
*/
1458-
private void verifyCheckSumForRead(final byte[] buffer, final AbfsHttpOperation result, final int bufferOffset)
1460+
private void verifyCheckSumForRead(final byte[] buffer, final String pathStr,
1461+
final AbfsHttpOperation result, final int bufferOffset)
14591462
throws AbfsRestOperationException{
14601463
// Number of bytes returned by server could be less than or equal to what
14611464
// caller requests. In case it is less, extra bytes will be initialized to 0
@@ -1475,10 +1478,10 @@ private void verifyCheckSumForRead(final byte[] buffer, final AbfsHttpOperation
14751478
String md5HashComputed = Base64.getEncoder().encodeToString(md5Bytes);
14761479
String md5HashActual = result.getResponseHeader(CONTENT_MD5);
14771480
if (!md5HashComputed.equals(md5HashActual)) {
1478-
throw new InvalidChecksumException(null);
1481+
throw new InvalidChecksumException(new PathIOException(pathStr));
14791482
}
14801483
} catch (NoSuchAlgorithmException e) {
1481-
throw new InvalidChecksumException(e);
1484+
throw new InvalidChecksumException(new PathIOException(pathStr));
14821485
}
14831486
}
14841487

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ protected AbstractAbfsIntegrationTest() throws Exception {
144144
} else {
145145
this.isIPAddress = false;
146146
}
147+
148+
// For tests, we want to enforce checksum validation so that any regressions can be caught.
149+
abfsConfig.setIsChecksumValidationEnabled(true);
147150
}
148151

149152
protected boolean getIsNamespaceEnabled(AzureBlobFileSystem fs)

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemChecksum.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.hadoop.conf.Configuration;
2929
import org.apache.hadoop.fs.FSDataInputStream;
3030
import org.apache.hadoop.fs.FSDataOutputStream;
31+
import org.apache.hadoop.fs.FileSystem;
3132
import org.apache.hadoop.fs.Path;
3233
import org.apache.hadoop.fs.azurebfs.contracts.services.AppendRequestParameters;
3334
import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
@@ -68,7 +69,7 @@ public void testAppendWithChecksumAtDifferentOffsets() throws Exception {
6869

6970
@Test
7071
public void testReadWithChecksumAtDifferentOffsets() throws Exception {
71-
AzureBlobFileSystem fs = getConfiguredFileSystem(4* ONE_MB, 4 * ONE_MB, true);
72+
AzureBlobFileSystem fs = getConfiguredFileSystem(4 * ONE_MB, 4 * ONE_MB, true);
7273
AbfsClient client = fs.getAbfsStore().getClient();
7374
Path path = path("testPath");
7475
fs.create(path);
@@ -123,7 +124,7 @@ private void testWriteReadWithChecksumInternal(final boolean readAheadEnabled)
123124
4 * ONE_MB, 4 * ONE_MB, readAheadEnabled);
124125
final int datasize = 16 * ONE_MB + 1000;
125126

126-
Path testPath = new Path("a/b.txt");
127+
Path testPath = path("testPath");
127128
byte[] bytesUploaded = generateRandomBytes(datasize);
128129
FSDataOutputStream out = fs.create(testPath);
129130
out.write(bytesUploaded);
@@ -152,7 +153,7 @@ private void testWriteReadWithChecksumAndOptionsInternal(
152153
8 * ONE_MB, ONE_MB, readAheadEnabled);
153154
final int datasize = 16 * ONE_MB + 1000;
154155

155-
Path testPath = new Path("a/b.txt");
156+
Path testPath = path("testPath");
156157
byte[] bytesUploaded = generateRandomBytes(datasize);
157158
FSDataOutputStream out = fs.create(testPath);
158159
out.write(bytesUploaded);
@@ -175,9 +176,8 @@ private void testWriteReadWithChecksumAndOptionsInternal(
175176

176177
private AzureBlobFileSystem getConfiguredFileSystem(final int writeBuffer,
177178
final int readBuffer, final boolean readAheadEnabled) throws Exception {
178-
AzureBlobFileSystem fs1 = getFileSystem();
179-
Configuration conf = fs1.getConf();
180-
AzureBlobFileSystem fs = getFileSystem(conf);
179+
Configuration conf = getRawConfiguration();
180+
AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(conf);
181181
AbfsConfiguration abfsConf = fs.getAbfsStore().getAbfsConfiguration();
182182
abfsConf.setIsChecksumValidationEnabled(true);
183183
abfsConf.setWriteBufferSize(writeBuffer);

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsInputStream.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ protected AzureBlobFileSystem getFileSystem(boolean readSmallFilesCompletely)
169169
final AzureBlobFileSystem fs = getFileSystem();
170170
getAbfsStore(fs).getAbfsConfiguration()
171171
.setReadSmallFilesCompletely(readSmallFilesCompletely);
172+
getAbfsStore(fs).getAbfsConfiguration()
173+
.setIsChecksumValidationEnabled(true);
172174
return fs;
173175
}
174176

@@ -177,6 +179,8 @@ private AzureBlobFileSystem getFileSystem(boolean optimizeFooterRead,
177179
final AzureBlobFileSystem fs = getFileSystem();
178180
getAbfsStore(fs).getAbfsConfiguration()
179181
.setOptimizeFooterRead(optimizeFooterRead);
182+
getAbfsStore(fs).getAbfsConfiguration()
183+
.setIsChecksumValidationEnabled(true);
180184
if (fileSize <= getAbfsStore(fs).getAbfsConfiguration()
181185
.getReadBufferSize()) {
182186
getAbfsStore(fs).getAbfsConfiguration()

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsInputStreamReadFooter.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,8 @@ private AzureBlobFileSystem getFileSystem(boolean optimizeFooterRead,
347347
final AzureBlobFileSystem fs = getFileSystem();
348348
getAbfsStore(fs).getAbfsConfiguration()
349349
.setOptimizeFooterRead(optimizeFooterRead);
350+
getAbfsStore(fs).getAbfsConfiguration()
351+
.setIsChecksumValidationEnabled(true);
350352
if (fileSize <= getAbfsStore(fs).getAbfsConfiguration()
351353
.getReadBufferSize()) {
352354
getAbfsStore(fs).getAbfsConfiguration()

0 commit comments

Comments
 (0)