You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2022/05/05 00:16:00 UTC
[jira] [Work logged] (HADOOP-17864) ABFS: Fork AbfsHttpOperation to add alternate connection
[ https://issues.apache.org/jira/browse/HADOOP-17864?focusedWorklogId=766351&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-766351 ]
ASF GitHub Bot logged work on HADOOP-17864:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 05/May/22 00:15
Start Date: 05/May/22 00:15
Worklog Time Spent: 10m
Work Description: raymondlam12 commented on code in PR #3335:
URL: https://github.com/apache/hadoop/pull/3335#discussion_r865463930
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java:
##########
@@ -20,33 +20,24 @@
import java.io.IOException;
import java.io.InputStream;
-import java.io.OutputStream;
-import java.net.HttpURLConnection;
import java.net.URL;
-import java.util.List;
+import java.util.HashMap;
-import javax.net.ssl.HttpsURLConnection;
-import javax.net.ssl.SSLSocketFactory;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
import org.apache.hadoop.fs.azurebfs.utils.UriUtils;
-import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory;
-import org.codehaus.jackson.JsonFactory;
-import org.codehaus.jackson.JsonParser;
-import org.codehaus.jackson.JsonToken;
-import org.codehaus.jackson.map.ObjectMapper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
-import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;
import org.apache.hadoop.fs.azurebfs.contracts.services.AbfsPerfLoggable;
-import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultSchema;
/**
* Represents an HTTP operation.
*/
-public class AbfsHttpOperation implements AbfsPerfLoggable {
+public abstract class AbfsHttpOperation implements AbfsPerfLoggable {
Review Comment:
Thoughts on creating an abstract class called AbfsOperation and creating AbfsHttpOperation / AbfsFastpathOperation subclasses?
The naming convention is a bit confusing to me right now with AbfsHttpOperation, AbfsHttpConnection & AbfsFastpathConnection.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpConnection.java:
##########
@@ -0,0 +1,367 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLSocketFactory;
+
+import org.codehaus.jackson.JsonFactory;
+import org.codehaus.jackson.JsonParser;
+import org.codehaus.jackson.JsonToken;
+import org.codehaus.jackson.map.ObjectMapper;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory;
+import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
+import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;
+import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultSchema;
+
+public class AbfsHttpConnection extends AbfsHttpOperation {
+ private static final Logger LOG = LoggerFactory.getLogger(AbfsHttpOperation.class);
+ private HttpURLConnection connection;
+ private ListResultSchema listResultSchema = null;
+
+ public AbfsHttpConnection(final URL url,
+ final String method,
+ List<AbfsHttpHeader> requestHeaders) throws IOException {
+ super(url, method);
+ init(method, requestHeaders);
+ }
+
+ /**
+ * Initializes a new HTTP request and opens the connection.
+ *
+ * @param method The HTTP method (PUT, PATCH, POST, GET, HEAD, or DELETE).
+ * @param requestHeaders The HTTP request headers.READ_TIMEOUT
+ *
+ * @throws IOException if an error occurs.
+ */
+ public void init(final String method, List<AbfsHttpHeader> requestHeaders)
Review Comment:
Are users expected to call this or is this just internal helper for the constructor?
If so, could this be a private method?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java:
##########
@@ -159,12 +163,114 @@ public long getBytesReceived() {
return bytesReceived;
}
- public ListResultSchema getListResultSchema() {
- return listResultSchema;
+
+ protected void setStatusCode(final int statusCode) {
+ this.statusCode = statusCode;
+ }
+
+ protected void setStatusDescription(final String statusDescription) {
+ this.statusDescription = statusDescription;
+ }
+
+ protected void setStorageErrorCode(final String storageErrorCode) {
+ this.storageErrorCode = storageErrorCode;
+ }
+
+ protected void setStorageErrorMessage(final String storageErrorMessage) {
+ this.storageErrorMessage = storageErrorMessage;
+ }
+
+ protected void setRequestId(final String requestId) {
+ this.requestId = requestId;
+ }
+
+ protected void setBytesSent(final int bytesSent) {
+ this.bytesSent = bytesSent;
+ }
+
+ protected void setBytesReceived(final long bytesReceived) {
+ this.bytesReceived = bytesReceived;
+ }
+
+ protected void setRecvResponseTimeMs(final long recvResponseTimeMs) {
+ this.recvResponseTimeMs = recvResponseTimeMs;
+ }
+
+ protected long getRecvResponseTimeMs() {
+ return this.recvResponseTimeMs;
+ }
+
+ protected void setAuthType(final org.apache.hadoop.fs.azurebfs.services.AuthType authType) {
+ this.authType = authType;
+ }
+
+ protected void setAuthToken(final String authToken) {
+ this.authToken = authToken;
+ }
+
+ protected void setResponseContentBuffer(final byte[] responseContentBuffer) {
+ this.responseContentBuffer = responseContentBuffer;
+ }
+
+ protected void setAbfsHttpHeaders(final List<AbfsHttpHeader> abfsHttpHeaders) {
+ this.abfsHttpHeaders = abfsHttpHeaders;
+ }
+
+ protected List<AbfsHttpHeader> getAbfsHttpHeaders() {
+ return abfsHttpHeaders;
+ }
+
+ protected AbfsRestOperationType getOpType() {
+ return opType;
+ }
+
+ protected URL getUrl() {
+ return url;
+ }
+
+ protected boolean isTraceEnabled() {
+ return isTraceEnabled;
}
- public String getResponseHeader(String httpHeader) {
- return connection.getHeaderField(httpHeader);
+ protected void setConnectionTimeMs(final long connectionTimeMs) {
+ this.connectionTimeMs = connectionTimeMs;
+ }
+
+ protected void setSendRequestTimeMs(final long sendRequestTimeMs) {
+ this.sendRequestTimeMs = sendRequestTimeMs;
+ }
+
+ protected void setExpectedAppendPos(final String expectedAppendPos) {
Review Comment:
Following on the previous comment of these properties, is this expectedAppendPos property used in all AbfsHttpOperation, namely Http and Fastpath connections?
If not, this should be bubbled up to the AbfsHttpConnection.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java:
##########
@@ -159,12 +163,114 @@ public long getBytesReceived() {
return bytesReceived;
}
- public ListResultSchema getListResultSchema() {
- return listResultSchema;
+
+ protected void setStatusCode(final int statusCode) {
Review Comment:
Should all these properties reside in the base class?
Are they used in the AbfsFastpathConnection?
If not, I think the property should be bubbled up into the AbfsHttpConnection class.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -96,6 +97,17 @@ String getSasToken() {
return sasToken;
}
+ public ListResultSchema getListResultSchema()
+ throws AzureBlobFileSystemException {
+ if (result instanceof AbfsHttpConnection) {
+ return ((AbfsHttpConnection) this.result).getListResultSchema();
+ } else {
+ throw new AbfsRestOperationException(-1, null,
Review Comment:
Do we need to throw an exception here?
By default, when this field is unset, we would just return null.
Should we follow similar behaviour?
Do we expect this exception to ever be thrown?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpConnection.java:
##########
@@ -0,0 +1,367 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLSocketFactory;
+
+import org.codehaus.jackson.JsonFactory;
+import org.codehaus.jackson.JsonParser;
+import org.codehaus.jackson.JsonToken;
+import org.codehaus.jackson.map.ObjectMapper;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory;
+import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
+import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;
+import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultSchema;
+
+public class AbfsHttpConnection extends AbfsHttpOperation {
Review Comment:
This looks like a cut + paste of the code in AbfsHttpOperation.java .
Is that right?
And if not, could you point me to the code blocks that have changed?
Issue Time Tracking
-------------------
Worklog Id: (was: 766351)
Time Spent: 2h (was: 1h 50m)
> ABFS: Fork AbfsHttpOperation to add alternate connection
> --------------------------------------------------------
>
> Key: HADOOP-17864
> URL: https://issues.apache.org/jira/browse/HADOOP-17864
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Affects Versions: 3.4.0
> Reporter: Sneha Vijayarajan
> Assignee: Sneha Vijayarajan
> Priority: Major
> Labels: pull-request-available
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This Jira is to facilitate upcoming work as part of adding an alternate connection :
> [HADOOP-17853] ABFS: Enable optional store connectivity over azure specific protocol for data egress - ASF JIRA (apache.org)
> The scope of the change is to make AbfsHttpOperation as abstract class and create a child class AbfsHttpConnection. Future connection types will be added as child of AbfsHttpOperation.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org