Skip to content
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

Upgrade Azure SDK and Jackson (#72833) #72995

Merged
merged 16 commits into from
May 27, 2021
Merged

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented May 12, 2021

This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214

original PR #72833

This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214
@rjernst rjernst added :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs :Core/Infra/Core Core issues without another label >upgrade v8.0.0 v7.14.0 labels May 12, 2021
@elasticmachine elasticmachine added Team:Core/Infra Meta label for core/infra team Team:Distributed Meta label for distributed team (obsolete) labels May 12, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@rjernst
Copy link
Member Author

rjernst commented May 13, 2021

This PR is currently blocked on finding a way to not use the netty dns resolver. That (statically) tries to access /etc/hosts, which we do not allow (file reads outside of ES).

@fcofdez
Copy link
Contributor

fcofdez commented May 25, 2021

We were hitting this issue Azure/azure-sdk-for-java#21163 in our tests.

diff --git a/plugins/repository-azure/build.gradle b/plugins/repository-azure/build.gradle
index be1e164cb54..6fca701f359 100644
--- a/plugins/repository-azure/build.gradle
+++ b/plugins/repository-azure/build.gradle
@@ -22,8 +22,8 @@ esplugin {
 }

 versions << [
-  'azure': '12.11.0',
-  'azureCore': '1.15.0',
+  'azure': '12.11.1',
+  'azureCore': '1.16.0',
   'azureCoreHttpNetty': '1.9.1',
   'azureAvro': '12.0.3',

With these versions we should be good to go.
Could you add me as a reviewer once the PR is ready?

@rjernst
Copy link
Member Author

rjernst commented May 26, 2021

@fcofdez I updated as you suggested. There is still a failure in encrypted repository.

@fcofdez
Copy link
Contributor

fcofdez commented May 26, 2021

@rjernst I've pushed a fix in fcofdez@a6b70eb

Before this change we were creating a new netty http client instance per invocation as the client is stateless but with the new async DNS resolvers it was creating a new one each time. That lead to hit the limit imposed in https://github.com/AdoptOpenJDK/openjdk-jdk/blob/ff4997014fe5462dca2b313f3f483400ffee5b62/src/java.base/share/classes/sun/net/ResourceManager.java#L68
Now we only create a netty http client per azure repository client, this solves the problem but we should document this limitation and how to solve it for cases where more than 25 clients are used.

@rjernst
Copy link
Member Author

rjernst commented May 27, 2021

Thanks @fcofdez, that worked. CI is passing now, can you take another look at the whole PR?

@fcofdez fcofdez self-requested a review May 27, 2021 07:41
@@ -14,4 +14,6 @@ grant {
// Used by jackson bean deserialization
permission java.lang.RuntimePermission "accessDeclaredMembers";
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
// needed by netty dns resolver even though we don't use it (!)
permission java.io.FilePermission "/etc/hosts", "read";
Copy link
Contributor

@fcofdez fcofdez May 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the persmissions and opened a followup (#73479) to see how we might avoid using the netty dns resolver altogether.

Copy link
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@rjernst rjernst merged commit 6c4c4a0 into elastic:master May 27, 2021
@rjernst rjernst deleted the azure_upgrade2 branch May 27, 2021 14:55
rjernst added a commit to rjernst/elasticsearch that referenced this pull request May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214

Co-authored-by: Francisco Fernández Castaño <[email protected]>
rjernst added a commit that referenced this pull request May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214

backport #72995
backport #73011

Co-authored-by: Francisco Fernández Castaño <[email protected]>
Co-authored-by: Mark Vieira <[email protected]>
limingnihao added a commit to limingnihao/elasticsearch that referenced this pull request May 28, 2021
* master: (1643 commits)
  Make DataStreamsSnapshotsIT resilient to failures because of local time. (elastic#73516)
  Upgrade netty to 4.1.63 (elastic#73011)
  [DOCS] Create a new page for dissect content in scripting docs (elastic#73437)
  Deprecate freeze index API (elastic#72618)
  [DOCS] Remove 'closed data stream' reference
  [DOCS] Update alias references (elastic#73427)
  [DOCS]  Create a new page for grok content in scripting docs (elastic#73118)
  Remove dependency on azure shadowjar since it's no longer required
  [DOCS] Update backport policy for known issues (elastic#73489)
  Shadowed dependencies should be hidden from pom dependencies (elastic#73467)
  Disable transitive dependencies when resolving bwc JDBC driver artifact (elastic#73448)
  Print full JVM implementation version at start of build (elastic#73439)
  [DOCS] Update snapshot/restore for data stream aliases (elastic#73438)
  Upgrade Azure SDK and Jackson (elastic#72833) (elastic#72995)
  [DOCS] Fix typo (elastic#73337) (elastic#73474)
  [DOCS] Fix typo (elastic#73444) (elastic#73472)
  [DOCS] Update alias security for data stream aliases (elastic#73436)
  Fix Bug with Concurrent Snapshot and Index Delete (elastic#73456)
  [DOCS] Move common scripting use cases up a level (elastic#73445)
  Add more validation for data stream aliases. (elastic#73416)
  ...
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Jun 7, 2021
The recent upgrade of the Azure SDK has caused a few test failures that
have been difficult to debug and do not yet have a fix. In particular, a
change to the netty reactor resolving
(reactor/reactor-netty#1655). We need to wait
for a fix for that issue, so this reverts commit
6c4c4a0.

relates elastic#73493
rjernst added a commit that referenced this pull request Jun 7, 2021
The recent upgrade of the Azure SDK has caused a few test failures that
have been difficult to debug and do not yet have a fix. In particular, a
change to the netty reactor resolving
(reactor/reactor-netty#1655). We need to wait
for a fix for that issue, so this reverts commit
6c4c4a0.

relates #73493
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Jun 7, 2021
…elastic#73493)"

The recent upgrade of the Azure SDK has caused a few test failures that
have been difficult to debug and do not yet have a fix. In particular, a
change to the netty reactor resolving
(reactor/reactor-netty#1655). We need to wait
for a fix for that issue, so this reverts commit
f454cef.

relates elastic#73493
rjernst added a commit that referenced this pull request Jun 8, 2021
…3861)

The recent upgrade of the Azure SDK has caused a few test failures that
have been difficult to debug and do not yet have a fix. In particular, a
change to the netty reactor resolving
(reactor/reactor-netty#1655). We need to wait
for a fix for that issue, so this reverts commit
f454cef.

relates #73493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Core/Infra Meta label for core/infra team Team:Distributed Meta label for distributed team (obsolete) >upgrade v7.14.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hex numbers in YAML requests are treated as strings
4 participants