anujmodi2021 commented on code in PR #6617:
URL: https://github.com/apache/hadoop/pull/6617#discussion_r1519176063
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsInputStreamReadFooter.java:
##########
@@ -71,22 +94,46 @@ public void
testMultipleServerCallsAreMadeWhenTheConfIsFalse()
private void testNumBackendCalls(boolean optimizeFooterRead)
throws Exception {
int fileIdx = 0;
+ final List<Future> futureList = new ArrayList<>();
+ for (int i = 0; i <= 4; i++) {
+ final int fileSize = (int) Math.pow(2, i) * SIZE_256_KB;
+ final int fileId = fileIdx++;
+ Future future = executorService.submit(() -> {
+ try {
+ try (AzureBlobFileSystem spiedFs = createSpiedFs(
Review Comment:
Why do we need a try inside try?
If we are catching a general exception, can we have a single exception?
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsInputStreamReadFooter.java:
##########
@@ -443,27 +575,35 @@ private FutureDataInputStreamBuilder
getParameterizedBuilder(final Path path,
return builder;
}
- private AzureBlobFileSystem getFileSystem(final boolean optimizeFooterRead,
- final int fileSize) throws IOException {
- final AzureBlobFileSystem fs = getFileSystem();
- AzureBlobFileSystemStore store = getAbfsStore(fs);
- store.getAbfsConfiguration().setOptimizeFooterRead(optimizeFooterRead);
- if (fileSize <= store.getAbfsConfiguration().getReadBufferSize()) {
- store.getAbfsConfiguration().setReadSmallFilesCompletely(false);
+ private void changeFooterConfigs(final AzureBlobFileSystem spiedFs,
+ final boolean optimizeFooterRead, final int fileSize,
+ final int readBufferSize) throws IOException {
+ AbfsConfiguration configuration =
spiedFs.getAbfsStore().getAbfsConfiguration();
+
Mockito.doReturn(optimizeFooterRead).when(configuration).optimizeFooterRead();
+ if (fileSize <= readBufferSize) {
+ Mockito.doReturn(false).when(configuration).readSmallFilesCompletely();
}
- return fs;
}
- private AzureBlobFileSystem getFileSystem(final boolean optimizeFooterRead,
- final int fileSize, final int footerReadBufferSize) throws IOException {
- final AzureBlobFileSystem fs = getFileSystem();
- AzureBlobFileSystemStore store = getAbfsStore(fs);
- store.getAbfsConfiguration().setOptimizeFooterRead(optimizeFooterRead);
- store.getAbfsConfiguration().setFooterReadBufferSize(footerReadBufferSize);
- if (fileSize <= store.getAbfsConfiguration().getReadBufferSize()) {
- store.getAbfsConfiguration().setReadSmallFilesCompletely(false);
+ private AzureBlobFileSystem createSpiedFs(Configuration configuration)
throws IOException {
+ AzureBlobFileSystem spiedFs = Mockito.spy((AzureBlobFileSystem)
FileSystem.newInstance(configuration));
+ AzureBlobFileSystemStore store = Mockito.spy(spiedFs.getAbfsStore());
+ Mockito.doReturn(store).when(spiedFs).getAbfsStore();
+ AbfsConfiguration spiedConfig = Mockito.spy(store.getAbfsConfiguration());
+ Mockito.doReturn(spiedConfig).when(store).getAbfsConfiguration();
+ return spiedFs;
+ }
+
+ private void changeFooterConfigs(final AzureBlobFileSystem spiedFs,
+ final boolean optimizeFooterRead, final int fileSize,
+ final int footerReadBufferSize, final int readBufferSize) throws
IOException {
Review Comment:
nit: IOException is never thrown in the method body, can be avoided.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsInputStreamReadFooter.java:
##########
@@ -167,24 +214,55 @@ public void testSeekToEndAndReadWithConfFalse() throws
Exception {
private void testSeekAndReadWithConf(boolean optimizeFooterRead,
SeekTo seekTo) throws Exception {
+ int fileIdx = 0;
+ List<Future> futureList = new ArrayList<>();
+ for (int j = 0; j <= 4; j++) {
+ final int fileSize = (int) Math.pow(2, j) * SIZE_256_KB;
+ final int fileId = fileIdx++;
+ futureList.add(executorService.submit(() -> {
+ try {
+ try (AzureBlobFileSystem spiedFs = createSpiedFs(
Review Comment:
Multiple try here as well.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsInputStreamReadFooter.java:
##########
@@ -167,24 +214,55 @@ public void testSeekToEndAndReadWithConfFalse() throws
Exception {
private void testSeekAndReadWithConf(boolean optimizeFooterRead,
SeekTo seekTo) throws Exception {
+ int fileIdx = 0;
+ List<Future> futureList = new ArrayList<>();
+ for (int j = 0; j <= 4; j++) {
+ final int fileSize = (int) Math.pow(2, j) * SIZE_256_KB;
+ final int fileId = fileIdx++;
+ futureList.add(executorService.submit(() -> {
+ try {
+ try (AzureBlobFileSystem spiedFs = createSpiedFs(
+ getRawConfiguration())) {
+ String fileName = methodName.getMethodName() + fileId;
+ byte[] fileContent = getRandomBytesArray(fileSize);
+ Path testFilePath = createFileWithContent(spiedFs, fileName,
+ fileContent);
+ for (int i = 0; i <= 4; i++) {
Review Comment:
I was wondering that since we are now changing the read buffer size as well.
Do we really need file size and read buffer size to have 5 different
values...
Objective here is to test all possible scenarios of relative values of three
sizes (File size, Read buffer size, Footer read buffer size)
So, permutations of three values of three sizes can be used to achieve that.
All the three sizes can range in set [256KB, 512KB, 1MB] and we should be
good to go.
Earlier file sizes were made to go till 4MB because read buffer size was
kept at default 4MB.
This will reduce the test runtime as well.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
