-
Notifications
You must be signed in to change notification settings - Fork 413
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
OAK-8413 Use the new Azure SDK in the Azure Segment Store #1748
base: trunk
Are you sure you want to change the base?
Conversation
…to issue/OAK-8413 # Conflicts: # oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/tool/SegmentCopy.java # oak-segment-azure/src/test/java/org/apache/jackrabbit/oak/segment/azure/AzureSegmentStoreServiceTest.java
@ierandra, the build fails with the following error in Oak Segment Azure: |
@dulceanu I was using |
@ierandra, yes, the versions need to be updated so that the build passes without skipping the baseline check. |
… package versions
Now |
@dulceanu I fixed all the tests. build should be ok now. |
This doesn't seem to compile (after applying the diff to oak trunk):
...etc... |
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.
See #1748 (comment)
…to issue/OAK-8413 # Conflicts: # oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureArchiveManager.java # oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureJournalFile.java # oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/tool/ToolUtils.java
@reschke I updated the PR with changes from apache/jackrabbit-oak trunk and should be fine now |
It does indeed; thanks. |
This looks a bit weird:
are we exporting these packages? This looks wrong...:
|
</Export-Package> | ||
<Embed-Dependency> | ||
azure-storage, | ||
azure-keyvault-core, | ||
azure-core, | ||
azure-identity, | ||
azure-json, | ||
azure-xml, |
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.
Arer we embedding the new SDK? Why?
(We embedded the old one due to the Guava dependency issue, but this should not be needed here)
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.
I tried without and I got the following error at build:
[ERROR] Bundle oak-segment-azure:1.71-ierandra-T20241104112150-3ccdb109e0 is importing package(s) com.azure.xml in start level 15 but no bundle is exporting these for that start level.
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.
In the oak-it-osgi tests? In which case we may have to add the dependency there.
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.
Opened https://issues.apache.org/jira/browse/OAK-11236 - the embedding/exporting issues started earlier. |
…refix and missing metadata
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.
As this adds many new files, please make sure that all of them just use LF as line ends (otherwise fixing this will cause ugly diffs), and check with "-Prat" that all licenses are ok.
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/util/package-info.java
Show resolved
Hide resolved
The build on my machine fails because of baseline plugin
|
@smiroslav I configure it back to 2.0.0. should work now |
...ure/src/main/java/org/apache/jackrabbit/oak/segment/azure/v8/AzureSegmentStoreServiceV8.java
Outdated
Show resolved
Hide resolved
...ure/src/main/java/org/apache/jackrabbit/oak/segment/azure/v8/AzureSegmentStoreServiceV8.java
Outdated
Show resolved
Hide resolved
...ure/src/main/java/org/apache/jackrabbit/oak/segment/azure/v8/AzureSegmentStoreServiceV8.java
Outdated
Show resolved
Hide resolved
...ure/src/main/java/org/apache/jackrabbit/oak/segment/azure/v8/AzureSegmentStoreServiceV8.java
Outdated
Show resolved
Hide resolved
...ure/src/main/java/org/apache/jackrabbit/oak/segment/azure/v8/AzureSegmentStoreServiceV8.java
Outdated
Show resolved
Hide resolved
...ent-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzurePersistenceManager.java
Show resolved
Hide resolved
...ure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureHttpRequestLoggingPolicy.java
Show resolved
Hide resolved
requestOptions.setMaximumExecutionTimeInMs(LEASE_RENEWAL_TIMEOUT_MS); | ||
requestOptions.setRetryPolicyFactory(new RetryNoRetry()); | ||
blob.renewLease(AccessCondition.generateLeaseCondition(leaseId), requestOptions, null); | ||
leaseId = leaseClient.renewLeaseWithResponse((RequestConditions) null, Duration.ofMillis(LEASE_RENEWAL_TIMEOUT_MS), Context.NONE).getValue(); |
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.
The previous implementation configured request options with RetryNoRetry
.
How is it achieved here?
...segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java
Outdated
Show resolved
Hide resolved
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/tool/AzureCheck.java
Show resolved
Hide resolved
...nt-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/util/AzureRequestOptions.java
Outdated
Show resolved
Hide resolved
...nt-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/util/AzureRequestOptions.java
Outdated
Show resolved
Hide resolved
...nt-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/util/AzureRequestOptions.java
Outdated
Show resolved
Hide resolved
return new RequestRetryOptions(RetryPolicyType.EXPONENTIAL, | ||
retryAttempts, | ||
timeoutExecution, | ||
timeoutIntervalToMs, |
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.
the intent of TIMEOUT_INTERVAL_PROP
was to specify max execution time for one retry
From the Java documentation
Sets the timeout to use when making this request.
The server timeout interval begins at the time that the complete request has been received by the service, and the server begins processing the response. If the timeout interval elapses before the response is returned to the client, the operation times out. The timeout interval resets with each retry, if the request is retried.
You can check the intent of the properties in the linked below nad map it to the corresponding ones in v12
No description provided.