-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-17590 ABFS: Introduce Lease Operations with Append to provide single writer semantics #3026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5048e01
ba8ca7f
fa3a9a5
117237e
be76592
f911986
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -506,13 +506,15 @@ public OutputStream createFile(final Path path, | |
| triggerConditionalCreateOverwrite = true; | ||
| } | ||
|
|
||
| AbfsLease lease = maybeCreateFiniteLease(relativePath, isNamespaceEnabled); | ||
| AbfsRestOperation op; | ||
| if (triggerConditionalCreateOverwrite) { | ||
| op = conditionalCreateOverwriteFile(relativePath, | ||
| statistics, | ||
| isNamespaceEnabled ? getOctalNotation(permission) : null, | ||
| isNamespaceEnabled ? getOctalNotation(umask) : null, | ||
| isAppendBlob | ||
| isAppendBlob, | ||
| lease | ||
| ); | ||
|
|
||
| } else { | ||
|
|
@@ -521,12 +523,14 @@ public OutputStream createFile(final Path path, | |
| isNamespaceEnabled ? getOctalNotation(permission) : null, | ||
| isNamespaceEnabled ? getOctalNotation(umask) : null, | ||
| isAppendBlob, | ||
| null); | ||
| null, | ||
| lease); | ||
| } | ||
| perfInfo.registerResult(op.getResult()).registerSuccess(true); | ||
|
|
||
| AbfsLease lease = maybeCreateLease(relativePath); | ||
|
|
||
| if (lease == null) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acquiring lease along with create/append enables atomic fetch of lease with the store operation and prevent any parallel writers. If configured for infinite lease, the path creation will happen without bundled lease acquiry and rely on later acquireLease API call in AbfsLease. Infinite lease flow should purely be on acquireLease API only if fs.azure.write.enforceLease is off. |
||
| lease = maybeCreateLease(relativePath, isNamespaceEnabled); | ||
| } | ||
| return new AbfsOutputStream( | ||
| client, | ||
| statistics, | ||
|
|
@@ -551,15 +555,16 @@ private AbfsRestOperation conditionalCreateOverwriteFile(final String relativePa | |
| final FileSystem.Statistics statistics, | ||
| final String permission, | ||
| final String umask, | ||
| final boolean isAppendBlob) throws AzureBlobFileSystemException { | ||
| final boolean isAppendBlob, | ||
| AbfsLease lease) throws AzureBlobFileSystemException { | ||
| AbfsRestOperation op; | ||
|
|
||
| try { | ||
| // Trigger a create with overwrite=false first so that eTag fetch can be | ||
| // avoided for cases when no pre-existing file is present (major portion | ||
| // of create file traffic falls into the case of no pre-existing file). | ||
| op = client.createPath(relativePath, true, | ||
| false, permission, umask, isAppendBlob, null); | ||
| false, permission, umask, isAppendBlob, null, lease); | ||
| } catch (AbfsRestOperationException e) { | ||
| if (e.getStatusCode() == HttpURLConnection.HTTP_CONFLICT) { | ||
| // File pre-exists, fetch eTag | ||
|
|
@@ -583,7 +588,7 @@ private AbfsRestOperation conditionalCreateOverwriteFile(final String relativePa | |
| try { | ||
| // overwrite only if eTag matches with the file properties fetched befpre | ||
| op = client.createPath(relativePath, true, | ||
| true, permission, umask, isAppendBlob, eTag); | ||
| true, permission, umask, isAppendBlob, eTag, lease); | ||
| } catch (AbfsRestOperationException ex) { | ||
| if (ex.getStatusCode() == HttpURLConnection.HTTP_PRECON_FAILED) { | ||
| // Is a parallel access case, as file with eTag was just queried | ||
|
|
@@ -639,7 +644,7 @@ public void createDirectory(final Path path, final FsPermission permission, fina | |
| final AbfsRestOperation op = client.createPath(getRelativePath(path), | ||
| false, overwrite, | ||
| isNamespaceEnabled ? getOctalNotation(permission) : null, | ||
| isNamespaceEnabled ? getOctalNotation(umask) : null, false, null); | ||
| isNamespaceEnabled ? getOctalNotation(umask) : null, false, null, null); | ||
| perfInfo.registerResult(op.getResult()).registerSuccess(true); | ||
| } | ||
| } | ||
|
|
@@ -738,7 +743,7 @@ public OutputStream openFileForWrite(final Path path, final FileSystem.Statistic | |
| isAppendBlob = true; | ||
| } | ||
|
|
||
| AbfsLease lease = maybeCreateLease(relativePath); | ||
| AbfsLease lease = maybeCreateLease(relativePath, getIsNamespaceEnabled()); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make it only for hns enabled |
||
|
|
||
| return new AbfsOutputStream( | ||
| client, | ||
|
|
@@ -1698,14 +1703,29 @@ private void updateInfiniteLeaseDirs() { | |
| this.azureInfiniteLeaseDirSet.remove(""); | ||
| } | ||
|
|
||
| private AbfsLease maybeCreateLease(String relativePath) | ||
| private AbfsLease maybeCreateFiniteLease(String relativePath, boolean isNamespaceEnabled) | ||
| throws AzureBlobFileSystemException { | ||
| boolean enableInfiniteLease = isInfiniteLeaseKey(relativePath); | ||
| if (!enableInfiniteLease) { | ||
| return null; | ||
| AbfsLease lease = null; | ||
| if (!enableInfiniteLease && abfsConfiguration.isLeaseEnforced() && isNamespaceEnabled) { | ||
| lease = new AbfsLease(client, relativePath, false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there scenarios where a finite lease created needs to explicitly call on acquireLease API ? If that dependency isnt there, it would be better to create 2 child classes to AbfsLease as AbfsInfiniteLeaseV1 and AbfsApiBundledLease. |
||
| } | ||
| AbfsLease lease = new AbfsLease(client, relativePath); | ||
| leaseRefs.put(lease, null); | ||
|
|
||
| return lease; | ||
| } | ||
|
|
||
| private AbfsLease maybeCreateLease(String relativePath, boolean isNamespaceEnabled) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Merge maybeCreateLease and maybeCreateInfiniteLease |
||
| throws AzureBlobFileSystemException { | ||
| boolean enableInfiniteLease = isInfiniteLeaseKey(relativePath); | ||
| AbfsLease lease = null; | ||
| if (enableInfiniteLease) { | ||
| lease = new AbfsLease(client, relativePath, true); | ||
| leaseRefs.put(lease, null); | ||
| } | ||
| else if (abfsConfiguration.isLeaseEnforced() && isNamespaceEnabled) { | ||
| lease = new AbfsLease(client, relativePath, false); | ||
| } | ||
|
|
||
| return lease; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,7 +82,7 @@ public class AbfsClient implements Closeable { | |
|
|
||
| private final URL baseUrl; | ||
| private final SharedKeyCredentials sharedKeyCredentials; | ||
| private final String xMsVersion = "2019-12-12"; | ||
| private final String xMsVersion = "2020-08-04"; | ||
| private final ExponentialRetryPolicy retryPolicy; | ||
| private final String filesystem; | ||
| private final AbfsConfiguration abfsConfiguration; | ||
|
|
@@ -338,7 +338,8 @@ public AbfsRestOperation deleteFilesystem() throws AzureBlobFileSystemException | |
|
|
||
| public AbfsRestOperation createPath(final String path, final boolean isFile, final boolean overwrite, | ||
| final String permission, final String umask, | ||
| final boolean isAppendBlob, final String eTag) throws AzureBlobFileSystemException { | ||
| final boolean isAppendBlob, final String eTag, | ||
| AbfsLease lease) throws AzureBlobFileSystemException { | ||
| final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders(); | ||
| if (isFile) { | ||
| addCustomerProvidedKeyHeaders(requestHeaders); | ||
|
|
@@ -359,6 +360,12 @@ public AbfsRestOperation createPath(final String path, final boolean isFile, fin | |
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.IF_MATCH, eTag)); | ||
| } | ||
|
|
||
| if (lease != null && lease.getLeaseID() != null && !lease.getLeaseID().isEmpty() && isFile) { | ||
| lease.setLeaseAcquired(true); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_PROPOSED_LEASE_ID, lease.getLeaseID())); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_DURATION, String.valueOf(abfsConfiguration.getWriteLeaseDuration()))); | ||
| } | ||
|
|
||
| final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); | ||
| abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, isFile ? FILE : DIRECTORY); | ||
| if (isAppendBlob) { | ||
|
|
@@ -561,17 +568,14 @@ public AbfsRestOperation renameIdempotencyCheckOp( | |
| } | ||
|
|
||
| public AbfsRestOperation append(final String path, final byte[] buffer, | ||
| AppendRequestParameters reqParams, final String cachedSasToken) | ||
| AppendRequestParameters reqParams, final String cachedSasToken, AbfsLease lease) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lease should be a member of AppendRequestParameters. |
||
| throws AzureBlobFileSystemException { | ||
| final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders(); | ||
| addCustomerProvidedKeyHeaders(requestHeaders); | ||
| // JDK7 does not support PATCH, so to workaround the issue we will use | ||
| // PUT and specify the real method in the X-Http-Method-Override header. | ||
| requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE, | ||
| HTTP_METHOD_PATCH)); | ||
| if (reqParams.getLeaseId() != null) { | ||
| requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, reqParams.getLeaseId())); | ||
| } | ||
|
|
||
| final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); | ||
| abfsUriQueryBuilder.addQuery(QUERY_PARAM_ACTION, APPEND_ACTION); | ||
|
|
@@ -585,6 +589,26 @@ public AbfsRestOperation append(final String path, final byte[] buffer, | |
| } | ||
| } | ||
|
|
||
| if (lease != null && lease.getLeaseID() != null && !lease.getLeaseID().isEmpty()) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. redundant code in flush and append. Create a function
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add below member functions to AbfsLease :
|
||
| if (lease.isInfiniteLease()) { | ||
| requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, lease.getLeaseID())); | ||
| } else { | ||
| if (!lease.isLeaseAcquired()) { | ||
| lease.setLeaseAcquired(true); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ACTION, | ||
| reqParams.getMode() == AppendRequestParameters.Mode.FLUSH_CLOSE_MODE ? ACQUIRE_RELEASE_LEASE_ACTION : ACQUIRE_LEASE_ACTION)); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_PROPOSED_LEASE_ID, lease.getLeaseID())); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_DURATION, String.valueOf(abfsConfiguration.getWriteLeaseDuration()))); | ||
| } else if (reqParams.getMode() == AppendRequestParameters.Mode.FLUSH_CLOSE_MODE) { | ||
| lease.setLeaseAcquired(false); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ACTION, RELEASE_LEASE_ACTION)); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ID, lease.getLeaseID())); | ||
| } else { | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ACTION, AUTO_RENEW_LEASE_ACTION)); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ID, lease.getLeaseID())); | ||
| } | ||
| } | ||
| } | ||
| // AbfsInputStream/AbfsOutputStream reuse SAS tokens for better performance | ||
| String sasTokenForReuse = appendSASTokenToQuery(path, SASTokenProvider.WRITE_OPERATION, | ||
| abfsUriQueryBuilder, cachedSasToken); | ||
|
|
@@ -648,16 +672,33 @@ public boolean appendSuccessCheckOp(AbfsRestOperation op, final String path, | |
| } | ||
|
|
||
| public AbfsRestOperation flush(final String path, final long position, boolean retainUncommittedData, | ||
| boolean isClose, final String cachedSasToken, final String leaseId) | ||
| boolean isClose, final String cachedSasToken, AbfsLease lease) | ||
| throws AzureBlobFileSystemException { | ||
| final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders(); | ||
| addCustomerProvidedKeyHeaders(requestHeaders); | ||
| // JDK7 does not support PATCH, so to workaround the issue we will use | ||
| // PUT and specify the real method in the X-Http-Method-Override header. | ||
| requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE, | ||
| HTTP_METHOD_PATCH)); | ||
| if (leaseId != null) { | ||
| requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, leaseId)); | ||
|
|
||
| if (lease != null && lease.getLeaseID() != null && !lease.getLeaseID().isEmpty()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above |
||
| if (lease.isInfiniteLease()) { | ||
| requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, lease.getLeaseID())); | ||
| } else { | ||
| if (!lease.isLeaseAcquired()) { | ||
| lease.setLeaseAcquired(true); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ACTION, isClose ? ACQUIRE_RELEASE_LEASE_ACTION : ACQUIRE_LEASE_ACTION)); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_PROPOSED_LEASE_ID, lease.getLeaseID())); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_DURATION, String.valueOf(abfsConfiguration.getWriteLeaseDuration()))); | ||
| } else if (isClose) { | ||
| lease.setLeaseAcquired(false); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ACTION, RELEASE_LEASE_ACTION)); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ID, lease.getLeaseID())); | ||
| } else { | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ACTION, AUTO_RENEW_LEASE_ACTION)); | ||
| requestHeaders.add(new AbfsHttpHeader(HttpHeaderConfigurations.X_MS_LEASE_ID, lease.getLeaseID())); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the path qualifies for infinteLease as per config settings, this will still create a finite lease ?
Call to a method createLease() that will first check for infiniteLease setting before defaulting to finite lease (if write lease config is enabled), would be the expectation ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it will not, it returns a null object if path qualifies for infinite lease