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.

Sign up for GitHub

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

Jump to bottom

Fix indexer connectivity check by catching the correct exception type #14063

Merged
merged 6 commits into from Nov 28, 2022
Merged

Fix indexer connectivity check by catching the correct exception type #14063

merged 6 commits into from Nov 28, 2022

Conversation

thll
Copy link
Contributor

@thll thll commented Nov 25, 2022 •
edited

The connectivity check for ES [Edit: and OpenSearch] was handling the wrong exception so that all calls to #isConnected would result in an exception when the ES cluster was not reachable. This only affected Elasticsearch, not OpenSearch.

This PR fixes that by catching the correct exception type.

To see the effect, sever the connection to ES while the server is running.

Before the change, the server log will be filled with a lot messages from the IndexFieldTypePollerPeriodical like these:

2022-11-25 17:23:26,792 ERROR: org.graylog2.indexer.fieldtypes.IndexFieldTypePollerPeriodical - Uncaught exception in Periodical
org.graylog.shaded.elasticsearch7.org.elasticsearch.ElasticsearchException: An error occurred: 
	at org.graylog.storage.elasticsearch7.ElasticsearchClient.exceptionFrom(ElasticsearchClient.java:155) ~[classes/:?]
	at org.graylog.storage.elasticsearch7.ElasticsearchClient.execute(ElasticsearchClient.java:112) ~[classes/:?]
	at org.graylog.storage.elasticsearch7.ElasticsearchClient.execute(ElasticsearchClient.java:105) ~[classes/:?]
	at org.graylog.storage.elasticsearch7.ClusterAdapterES7.isConnected(ClusterAdapterES7.java:169) ~[classes/:?]
	at org.graylog2.indexer.cluster.Cluster.isConnected(Cluster.java:115) ~[classes/:?]
	at org.graylog2.indexer.fieldtypes.IndexFieldTypePollerPeriodical.doRun(IndexFieldTypePollerPeriodical.java:106) ~[classes/:?]
	at org.graylog2.plugin.periodical.Periodical.run(Periodical.java:94) [classes/:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [?:?]
	at java.util.concurrent.FutureTask.runAndReset$$$capture(FutureTask.java:305) [?:?]
	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java) [?:?]
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) [?:?]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
	at java.lang.Thread.run(Thread.java:833) [?:?]
Caused by: org.graylog.shaded.elasticsearch7.org.apache.http.ConnectionClosedException: Connection is closed
	at org.graylog.shaded.elasticsearch7.org.elasticsearch.client.RestClient.extractAndWrapCause(RestClient.java:839) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.elasticsearch.client.RestClient.performRequest(RestClient.java:259) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.elasticsearch.client.RestClient.performRequest(RestClient.java:246) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.elasticsearch.client.RestHighLevelClient.internalPerformRequest(RestHighLevelClient.java:1613) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.elasticsearch.client.RestHighLevelClient.performRequest(RestHighLevelClient.java:1583) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.elasticsearch.client.RestHighLevelClient.performRequestAndParseEntity(RestHighLevelClient.java:1553) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.elasticsearch.client.ClusterClient.health(ClusterClient.java:130) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.storage.elasticsearch7.ClusterAdapterES7.lambda$isConnected$11(ClusterAdapterES7.java:169) ~[classes/:?]
	at org.graylog.storage.elasticsearch7.ElasticsearchClient.execute(ElasticsearchClient.java:110) ~[classes/:?]
	... 12 more
Caused by: org.graylog.shaded.elasticsearch7.org.apache.http.ConnectionClosedException: Connection is closed
	at org.graylog.shaded.elasticsearch7.org.apache.http.nio.protocol.HttpAsyncRequestExecutor.endOfInput(HttpAsyncRequestExecutor.java:356) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.DefaultNHttpClientConnection.consumeInput(DefaultNHttpClientConnection.java:261) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.client.InternalIODispatch.onInputReady(InternalIODispatch.java:81) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.client.InternalIODispatch.onInputReady(InternalIODispatch.java:39) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.reactor.AbstractIODispatch.inputReady(AbstractIODispatch.java:114) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.reactor.BaseIOReactor.readable(BaseIOReactor.java:162) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.reactor.AbstractIOReactor.processEvent(AbstractIOReactor.java:337) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.reactor.AbstractIOReactor.processEvents(AbstractIOReactor.java:315) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.reactor.AbstractIOReactor.execute(AbstractIOReactor.java:276) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.reactor.BaseIOReactor.execute(BaseIOReactor.java:104) ~[elasticsearch7-7.9.1-0.jar:?]
	at org.graylog.shaded.elasticsearch7.org.apache.http.impl.nio.reactor.AbstractMultiworkerIOReactor$Worker.run(AbstractMultiworkerIOReactor.java:591) ~[elasticsearch7-7.9.1-0.jar:?]
	... 1 more

After the change, for the IndexFieldTypePollerPeriodical, there will be just a single line logged:

2022-11-25 17:27:45,425 INFO : org.graylog2.indexer.fieldtypes.IndexFieldTypePollerPeriodical - Cluster not connected yet, delaying index field type initialization until it is reachable.

Graylog Version: 5.1.0-SNAPSHOT

@thll thll marked this pull request as ready for review November 25, 2022 16:35
@thll thll requested a review from todvora November 25, 2022 16:35
dennisoelkers
dennisoelkers reviewed Nov 28, 2022
View reviewed changes
Copy link
Member

@dennisoelkers dennisoelkers left a comment

Choose a reason for hiding this comment

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

Why is this not affecting OpenSearch? From what I can see, the OS code throws a org.graylog.shaded.opensearch2.org.opensearch.OpensearchException, but a org.graylog2.indexer.OpensearchException is caught, just similar to the ES-code?

@janheise
Copy link
Contributor

janheise commented Nov 28, 2022 •
edited

@dennisoelkers I think the corresponding class for OpenSearch 2 needs a change, too. I probably did not find this, because I replaced the shaded imports with new ones. @thll Would you both mind if I push a change to this PR for OpenSearch?

@thll
Copy link
Contributor Author

thll commented Nov 28, 2022

@dennisoelkers I think the corresponding class for OpenSearch needs a change, too. I probably did not find this, because I replaced the shaded imports with new ones. @thll Would you both mind if I push a change to this PR for OpenSearch?

Not at all. Go ahead! I thought I tried it with OpenSearch and didn't see the issue, but something else probably was at play.

@janheise
Copy link
Contributor

I'll check with OpenSearch again. But from my experience so far, I'm very sure that changing the exception in the OpenSearch 2 adapter makes more sense than keeping it as it was because the adapter is the changed ES7 adapter

@thll
Copy link
Contributor Author

thll commented Nov 28, 2022

Why is this not affecting OpenSearch? From what I can see, the OS code throws a org.graylog.shaded.opensearch2.org.opensearch.OpensearchException, but a org.graylog2.indexer.OpensearchException is caught, just similar to the ES-code?

I know what I did wrong. I tested with OpenSearch 1.3.1 which apparently just goes through the ES code path and was therefore already fixed.

Tried again with OpenSearch 2 and can confirm that it was also broken.

@thll thll changed the title Fix ES connectivity check by catching the correct exception type Fix indexer connectivity check by catching the correct exception type Nov 28, 2022
@janheise janheise self-requested a review November 28, 2022 12:20
@janheise janheise merged commit 9bb27f5 into master Nov 28, 2022
@janheise janheise deleted the fix/es-connectivity-check branch November 28, 2022 12:21
thll added a commit that referenced this pull request Nov 28, 2022
@thll
Fix indexer connectivity check by catching the correct exception type (…
1cab8ea 
…#14063)

* Fix connectivity check by catching the correct exception type

* add changelog

* Fix typo

* added changes for OpenSearch Adapter

* add changelog

* Revert "add changelog"

This reverts commit 9c61eae.

Co-authored-by: Jan Heise <jan.heise@graylog.com>
(cherry picked from commit 9bb27f5)
@thll thll mentioned this pull request Nov 28, 2022
Merged
thll added a commit that referenced this pull request Nov 28, 2022
@thll
Fix indexer connectivity check by catching the correct exception type (…
9b970b1 
…#14063)

* Fix connectivity check by catching the correct exception type

* add changelog

* Fix typo

* added changes for OpenSearch Adapter

* add changelog

* Revert "add changelog"

This reverts commit 9c61eae.

Co-authored-by: Jan Heise <jan.heise@graylog.com>

(cherry picked from commit 9bb27f5)
@thll thll mentioned this pull request Nov 28, 2022
Merged
dennisoelkers pushed a commit that referenced this pull request Nov 29, 2022
@thll
Fix indexer connectivity check by catching the correct exception type (…
c489840 
…#14063) (#14076)

* Fix connectivity check by catching the correct exception type

* add changelog

* Fix typo

* added changes for OpenSearch Adapter

* add changelog

* Revert "add changelog"

This reverts commit 9c61eae.

Co-authored-by: Jan Heise <jan.heise@graylog.com>

(cherry picked from commit 9bb27f5)
janheise added a commit that referenced this pull request Nov 29, 2022
@thll @janheise
Fix indexer connectivity check by catching the correct exception type (…
34e3fad 
…#14063) (#14075)

* Fix connectivity check by catching the correct exception type

* add changelog

* Fix typo

* added changes for OpenSearch Adapter

* add changelog

* Revert "add changelog"

This reverts commit 9c61eae.

Co-authored-by: Jan Heise <jan.heise@graylog.com>
(cherry picked from commit 9bb27f5)

Co-authored-by: Jan Heise <jan.heise@graylog.com>
bernd pushed a commit that referenced this pull request Dec 9, 2022
@thll @janheise @bernd
Fix indexer connectivity check by catching the correct exception type (…
e2f2861 
…#14063)

* Fix connectivity check by catching the correct exception type

* add changelog

* Fix typo

* added changes for OpenSearch Adapter

* add changelog

* Revert "add changelog"

This reverts commit 9c61eae.

Co-authored-by: Jan Heise <jan.heise@graylog.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dennisoelkers dennisoelkers dennisoelkers approved these changes

@janheise janheise janheise approved these changes

@todvora todvora Awaiting requested review from todvora

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@thll @janheise @dennisoelkers