Improve Health Status of security-auditlog Index in Single Node Clusters #5030
Conversation
Signed-off-by: Ganesh Bombatkar <[email protected]>
Ganesh-RB
requested review from
cwperks,
DarshitChanpura,
derek-ho,
nibix,
peternied,
RyanL1997,
reta and
willyborankin
as code owners
src/main/java/org/opensearch/security/auditlog/sink/AbstractInternalOpenSearchSink.java
Outdated
Show resolved
Hide resolved
| .isExists(); | ||
| } | ||
|
|
||
| private boolean createIndexIfAbsent(String indexName){ |
There was a problem hiding this comment.
nit: this could be moved to common/utils class.
src/main/java/org/opensearch/security/auditlog/sink/AbstractInternalOpenSearchSink.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Ganesh Bombatkar <[email protected]>
Ganesh-RB
force-pushed
the
auditlog_index_health_single_node_case
branch
from
7d60d06 to
0bceb07
Compare
src/main/java/org/opensearch/security/auditlog/sink/AbstractInternalOpenSearchSink.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/auditlog/sink/AbstractInternalOpenSearchSink.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Ganesh Bombatkar <[email protected]>
| @@ -64,6 +88,12 @@ public boolean doStore(final AuditMessage msg, String indexName) { | |||
|
|
|||
| try (StoredContext ctx = threadPool.getThreadContext().stashContext()) { | |||
| try { | |||
| boolean ok = createIndexIfAbsent(indexName); | |||
| if (!ok) { | |||
| log.error("Server not acknowledge for creation of index {}", indexName); | |||
There was a problem hiding this comment.
nit: error message could be something like
Failed to create index [index_name]
| } | ||
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
|
|
||
| } | ||
|
|
||
| private boolean createIndexIfAbsent(String indexName) { |
There was a problem hiding this comment.
nit: cover this function in UTs
| log.info("Index {} created?: {}", indexName, ok); | ||
| return ok; | ||
| } catch (ResourceAlreadyExistsException resourceAlreadyExistsException) { | ||
| log.info("Index {} already exists", indexName); |
There was a problem hiding this comment.
Shouldn't lastUsedIndexName be set here?
What happens in the case of node reboot and the audit log index exists? lastUsedIndexName would be null when the node starts. Shouldn't this catch clause be used to populate it if it is null and the index already exists?
There was a problem hiding this comment.
FYI I experimented around with passing the clusterService to the internal opensearch sinks which can be used to check if an index exists without having to issue an Exists request. Take a look at the change on my fork cwperks#44. That change also shows an example of how to use the integration test framework in this repo to write an integration test for this case.
Description
Bug Fix: Improve Health Status of security-auditlog Index in Single Node Clusters
This pull request addresses an issue where the
security-auditlogindex was showing a yellow health status in single node clusters. The problem was caused by the use ofindexrequestdefault settings, which were not optimal for single node environments.Changes:
security-auditlogindex to:{ "index.number_of_shards": 1, "index.auto_expand_replicas": "0-1" }Issues Resolved
Testing
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.