[
https://issues.apache.org/jira/browse/HADOOP-19102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17825151#comment-17825151
]
ASF GitHub Bot commented on HADOOP-19102:
-----------------------------------------
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.
> [ABFS]: FooterReadBufferSize should not be greater than readBufferSize
> ----------------------------------------------------------------------
>
> Key: HADOOP-19102
> URL: https://issues.apache.org/jira/browse/HADOOP-19102
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Reporter: Pranav Saxena
> Assignee: Pranav Saxena
> Priority: Major
> Labels: pull-request-available
> Fix For: 3.4.0, 3.5.0
>
>
> The method `optimisedRead` creates a buffer array of size `readBufferSize`.
> If footerReadBufferSize is greater than readBufferSize, abfs will attempt to
> read more data than the buffer array can hold, which causes an exception.
> Change: To avoid this, we will keep footerBufferSize =
> min(readBufferSizeConfig, footerBufferSizeConfig)
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
