snvijaya commented on a change in pull request #3381:
URL: https://github.com/apache/hadoop/pull/3381#discussion_r707324115
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java
##########
@@ -369,58 +378,75 @@ public void processResponse(final byte[] buffer, final
int offset, final int len
startTime = System.nanoTime();
}
- if (statusCode >= HttpURLConnection.HTTP_BAD_REQUEST) {
- processStorageErrorResponse();
+ long totalBytesRead = 0;
+
+ try {
+ totalBytesRead = parseResponse(buffer, offset, length);
+ } finally {
if (this.isTraceEnabled) {
this.recvResponseTimeMs += elapsedTimeMs(startTime);
}
- this.bytesReceived =
this.connection.getHeaderFieldLong(HttpHeaderConfigurations.CONTENT_LENGTH, 0);
- } else {
- // consume the input stream to release resources
- int totalBytesRead = 0;
+ this.bytesReceived = totalBytesRead;
+ }
+ }
+ public long parseResponse(final byte[] buffer,
+ final int offset,
+ final int length) throws IOException {
+ if (statusCode >= HttpURLConnection.HTTP_BAD_REQUEST) {
+ processStorageErrorResponse();
+ return this.connection.getHeaderFieldLong(
+ HttpHeaderConfigurations.CONTENT_LENGTH, 0);
+ } else {
try (InputStream stream = this.connection.getInputStream()) {
if (isNullInputStream(stream)) {
- return;
+ return 0;
}
- boolean endOfStream = false;
- // this is a list operation and need to retrieve the data
- // need a better solution
- if (AbfsHttpConstants.HTTP_METHOD_GET.equals(this.method) && buffer ==
null) {
+ if (AbfsHttpConstants.HTTP_METHOD_GET.equals(this.method)
+ && buffer == null) {
parseListFilesResponse(stream);
} else {
- if (buffer != null) {
- while (totalBytesRead < length) {
- int bytesRead = stream.read(buffer, offset + totalBytesRead,
length - totalBytesRead);
- if (bytesRead == -1) {
- endOfStream = true;
- break;
- }
- totalBytesRead += bytesRead;
- }
- }
- if (!endOfStream && stream.read() != -1) {
- // read and discard
- int bytesRead = 0;
- byte[] b = new byte[CLEAN_UP_BUFFER_SIZE];
- while ((bytesRead = stream.read(b)) >= 0) {
- totalBytesRead += bytesRead;
- }
- }
+ return readDataFromStream(stream, buffer, offset, length);
}
- } catch (IOException ex) {
- LOG.warn("IO/Network error: {} {}: {}",
- method, getMaskedUrl(), ex.getMessage());
- LOG.debug("IO Error: ", ex);
- throw ex;
- } finally {
- if (this.isTraceEnabled) {
- this.recvResponseTimeMs += elapsedTimeMs(startTime);
+ }
+ }
+
+ return 0;
+ }
+
+ public long readDataFromStream(final InputStream stream,
+ final byte[] buffer,
+ final int offset,
+ final int length) throws IOException {
+ // consume the input stream to release resources
+ int totalBytesRead = 0;
+ boolean endOfStream = false;
+
+ if (buffer != null) {
Review comment:
Ideally never. Other than List and Read, server should not be sending
any content that the client isnt ready.
In case of List, buffer is not provided by the caller as the size is not
known, and is parsed and returned before reaching this method.
Buffer passed in here can not be null in case of read flow either, as the
null check happens before HttpRequest can be raised.
However this is an existing protective check in the code, hence retaining it.
--
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]
