Skip to content

Add undeclared Azure settings, modify test to exercise them#118634

Merged
nicktindall merged 5 commits intoelastic:mainfrom
nicktindall:add_missing_azure_settings
Dec 13, 2024
Merged

Add undeclared Azure settings, modify test to exercise them#118634
nicktindall merged 5 commits intoelastic:mainfrom
nicktindall:add_missing_azure_settings

Conversation

@nicktindall
Copy link
Contributor

@nicktindall nicktindall commented Dec 12, 2024

Declares some settings that were undeclared by the Azure repository plugin.

Specifically, this PR allows use of

  • repository.azure.http_client.event_loop_executor_thread_count
  • repository.azure.http_client.max_open_connections
  • repository.azure.http_client.connection_timeout
  • repository.azure.http_client.connection_max_idle_time
@nicktindall nicktindall added >non-issue :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Dec 12, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. label Dec 12, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@nicktindall
Copy link
Contributor Author

I added tests that two of the settings are effective when set, the remaining ones don't seem to be easily testable

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

I think this should be labelled as >bug instead? It has a visible impact and also could use a doc update.

Being able to registering all these settings is already a test. So I am not too worried about testing whether the remaining two getting picked up by the connectionProvider.

ThreadPool nodeThreadPool = internalCluster().getInstance(ThreadPool.class);
assertEquals(
EVENT_LOOP_THREAD_COUNT_SETTING.get(),
((EsThreadPoolExecutor) nodeThreadPool.executor(AzureRepositoryPlugin.NETTY_EVENT_LOOP_THREAD_POOL_NAME)).getMaximumPoolSize()
Copy link
Member

Choose a reason for hiding this comment

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

Nit: can this be

nodeThreadPool.info(AzureRepositoryPlugin.NETTY_EVENT_LOOP_THREAD_POOL_NAME).max()

?

Copy link
Contributor Author

@nicktindall nicktindall Dec 13, 2024

Choose a reason for hiding this comment

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

Yes, that's much nicer, thanks. Used in a0eaa92

@elasticsearchmachine
Copy link
Collaborator

Hi @nicktindall, I've created a changelog YAML for you.

@nicktindall nicktindall merged commit 344cf15 into elastic:main Dec 13, 2024
@nicktindall nicktindall deleted the add_missing_azure_settings branch December 13, 2024 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. v9.0.0

3 participants