Skip to content

Commit f968baa

Browse files
committed
Resolving Comments
1 parent 60f1c38 commit f968baa

File tree

4 files changed

+43
-5
lines changed

4 files changed

+43
-5
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,7 +1626,7 @@ public ListResponseData parseListPathResults(AbfsHttpOperation result, URI uri)
16261626
// Throw as it is to avoid multiple wrapping.
16271627
LOG.error("Unable to deserialize list results for Uri {}", uri != null ? uri.toString(): "NULL", ex);
16281628
throw ex;
1629-
} catch (IOException ex) {
1629+
} catch (Exception ex) {
16301630
LOG.error("Unable to get stream for list results for uri {}", uri != null ? uri.toString(): "NULL", ex);
16311631
throw new AbfsDriverException(ERR_BLOB_LIST_PARSING, ex);
16321632
}
@@ -1926,7 +1926,8 @@ private List<AbfsHttpHeader> getMetadataHeadersList(final Hashtable<String, Stri
19261926
* @param uri URI to be used for path conversion.
19271927
* @return List of entries after removing duplicates.
19281928
*/
1929-
private ListResponseData filterDuplicateEntriesAndRenamePendingFiles(
1929+
@VisibleForTesting
1930+
public ListResponseData filterDuplicateEntriesAndRenamePendingFiles(
19301931
BlobListResultSchema listResultSchema, URI uri) throws IOException {
19311932
List<FileStatus> fileStatuses = new ArrayList<>();
19321933
Map<Path, Integer> renamePendingJsonPaths = new HashMap<>();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,9 +1507,9 @@ public ListResponseData parseListPathResults(AbfsHttpOperation result, URI uri)
15071507
// Throw as it is to avoid multiple wrapping.
15081508
LOG.error("Unable to deserialize list results for Uri {}", uri != null ? uri.toString(): "NULL", ex);
15091509
throw ex;
1510-
} catch (IOException ex) {
1510+
} catch (Exception ex) {
15111511
LOG.error("Unable to deserialize list results for Uri {}", uri != null ? uri.toString(): "NULL", ex);
1512-
throw new AbfsDriverException(ex);
1512+
throw new AbfsDriverException(ERR_DFS_LIST_PARSING, ex);
15131513
}
15141514
}
15151515

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,13 @@ private void parseBlockListResponse(final InputStream stream) throws IOException
517517
blockIdList = client.parseBlockListResponse(stream);
518518
}
519519

520+
/**
521+
* Parse the list path response from the network stream and save response into a buffer.
522+
* @param stream Network InputStream.
523+
* @throws IOException if an error occurs while reading the stream.
524+
*/
520525
private void parseListPathResponse(final InputStream stream) throws IOException {
521-
if (stream == null || blockIdList != null) {
526+
if (stream == null || listResultData != null) {
522527
return;
523528
}
524529
listResultData = readDataFromStream(stream);

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import java.io.FileNotFoundException;
2222
import java.io.IOException;
23+
import java.net.SocketException;
2324
import java.net.SocketTimeoutException;
2425
import java.net.URL;
2526
import java.util.ArrayList;
@@ -31,6 +32,7 @@
3132

3233
import org.assertj.core.api.Assertions;
3334
import org.junit.Test;
35+
import org.mockito.Mock;
3436
import org.mockito.Mockito;
3537
import org.mockito.stubbing.Stubber;
3638

@@ -40,8 +42,10 @@
4042
import org.apache.hadoop.fs.FileStatus;
4143
import org.apache.hadoop.fs.LocatedFileStatus;
4244
import org.apache.hadoop.fs.Path;
45+
import org.apache.hadoop.fs.azurebfs.constants.AbfsServiceType;
4346
import org.apache.hadoop.fs.azurebfs.constants.FSOperationType;
4447
import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;
48+
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsDriverException;
4549
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
4650
import org.apache.hadoop.fs.azurebfs.services.AbfsBlobClient;
4751
import org.apache.hadoop.fs.azurebfs.services.AbfsClientHandler;
@@ -63,7 +67,10 @@
6367
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.TRUE;
6468
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_LIST_MAX_RESULTS;
6569
import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_METADATA_PREFIX;
70+
import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.COPY_BLOB_ABORTED;
71+
import static org.apache.hadoop.fs.azurebfs.services.AbfsErrors.ERR_BLOB_LIST_PARSING;
6672
import static org.apache.hadoop.fs.azurebfs.services.RenameAtomicity.SUFFIX;
73+
import static org.apache.hadoop.fs.azurebfs.services.RetryReasonConstants.CONNECTION_RESET_MESSAGE;
6774
import static org.apache.hadoop.fs.azurebfs.services.RetryReasonConstants.CONNECTION_TIMEOUT_JDK_MESSAGE;
6875
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs;
6976
import static org.apache.hadoop.fs.contract.ContractTestUtils.createFile;
@@ -176,6 +183,31 @@ public void testListPathTracingContext() throws Exception {
176183
Mockito.verify(spiedTracingContext, times(0)).constructHeader(any(), any(), any());
177184
}
178185

186+
@Test
187+
public void testListPathParsingFailure() throws Exception {
188+
assumeBlobServiceType();
189+
AzureBlobFileSystem spiedFs = Mockito.spy(getFileSystem());
190+
AzureBlobFileSystemStore spiedStore = Mockito.spy(spiedFs.getAbfsStore());
191+
AbfsBlobClient spiedClient = Mockito.spy(spiedStore.getClientHandler()
192+
.getBlobClient());
193+
Mockito.doReturn(spiedStore).when(spiedFs).getAbfsStore();
194+
Mockito.doReturn(spiedClient).when(spiedStore).getClient();
195+
196+
Mockito.doThrow(new SocketException(CONNECTION_RESET_MESSAGE)).when(spiedClient).filterDuplicateEntriesAndRenamePendingFiles(any(), any());
197+
List<FileStatus> fileStatuses = new ArrayList<>();
198+
AbfsDriverException ex = intercept(AbfsDriverException.class,
199+
() -> {
200+
spiedStore.listStatus(new Path("/"), "", fileStatuses,
201+
true, null, getTestTracingContext(spiedFs, true));
202+
});
203+
Assertions.assertThat(ex.getStatusCode())
204+
.describedAs("Expecting Network Error status code")
205+
.isEqualTo(-1);
206+
Assertions.assertThat(ex.getErrorMessage())
207+
.describedAs("Expecting COPY_ABORTED error code")
208+
.contains(ERR_BLOB_LIST_PARSING);
209+
}
210+
179211
/**
180212
* Creates a file, verifies that listStatus returns it,
181213
* even while the file is still open for writing.

0 commit comments

Comments
 (0)