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

fix bug Observability plugin calling createIndex from onNodeStarted m… #1885

Open
wants to merge 5 commits into
base: 2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,29 @@ jobs:
build-linux:
needs: Get-CI-Image-Tag
strategy:
# Run all jobs
fail-fast: false
matrix:
java: [11, 17, 21]
runs-on: ubuntu-latest
container:
# using the same image which is used by opensearch-build team to build the OpenSearch Distribution
# this image tag is subject to change as more dependencies and updates will arrive over time
image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }}
# need to switch to root so that github actions can install runner binary on container without permission issues.
options: --user root

env:
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }}

steps:
- uses: actions/checkout@v1
- name: Run start commands
run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }}

- uses: actions/checkout@v4

- name: Set up JDK ${{ matrix.java }}
uses: actions/setup-java@v1
uses: actions/setup-java@v4
with:
java-version: ${{ matrix.java }}

distribution: 'temurin'

- name: Run Backwards Compatibility Tests
run: |
echo "Running backwards compatibility tests ..."
Expand All @@ -56,7 +58,7 @@ jobs:
cp -r ./build/distributions/*.zip opensearch-observability-builds/

- name: Upload Artifacts
uses: actions/upload-artifact@v1
uses: actions/upload-artifact@v4
with:
name: opensearch-observability-ubuntu-latest
path: opensearch-observability-builds
Expand All @@ -71,11 +73,12 @@ jobs:
runs-on: ${{ matrix.os }}

steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v4

- name: Set up JDK ${{ matrix.java }}
uses: actions/setup-java@v1
uses: actions/setup-java@v4
with:
distribution: 'temurin'
java-version: ${{ matrix.java }}

- name: Build with Gradle
Expand All @@ -88,7 +91,7 @@ jobs:
cp -r ./build/distributions/*.zip opensearch-observability-builds/

- name: Upload Artifacts
uses: actions/upload-artifact@v1
uses: actions/upload-artifact@v4
with:
name: opensearch-observability-${{ matrix.os }}
path: opensearch-observability-builds
Original file line number Diff line number Diff line change
Expand Up @@ -96,32 +96,41 @@ internal object ObservabilityIndex : LifecycleListener() {
*/
@Suppress("TooGenericExceptionCaught")
private fun createIndex() {
if (!isIndexExists(INDEX_NAME)) {
val classLoader = ObservabilityIndex::class.java.classLoader
val indexMappingSource = classLoader.getResource(OBSERVABILITY_MAPPING_FILE_NAME)?.readText()!!
val indexSettingsSource = classLoader.getResource(OBSERVABILITY_SETTINGS_FILE_NAME)?.readText()!!
val request = CreateIndexRequest(INDEX_NAME)
.mapping(indexMappingSource, XContentType.YAML)
.settings(indexSettingsSource, XContentType.YAML)
try {
val actionFuture = client.admin().indices().create(request)
val response = actionFuture.actionGet(PluginSettings.operationTimeoutMs)
if (response.isAcknowledged) {
log.info("$LOG_PREFIX:Index $INDEX_NAME creation Acknowledged")
reindexNotebooks()
} else {
error("$LOG_PREFIX:Index $INDEX_NAME creation not Acknowledged")
}
} catch (exception: ResourceAlreadyExistsException) {
log.warn("message: ${exception.message}")
} catch (exception: Exception) {
if (exception.cause !is ResourceAlreadyExistsException) {
throw exception
try {
// wait for the cluster here until it will finish managed node election
while (clusterService.state().blocks().hasGlobalBlockWithStatus(RestStatus.SERVICE_UNAVAILABLE)) {
log.info("Wait for cluster to be available ...")
Comment on lines +101 to +102
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the fix!
I had the same question that was raised by Shwetha in the original issue:

 The main branch doesn't have same code as well? why is there divergence?

Do we know why was this only needed for 2.x and what's the path ahead to converge these branches?

Copy link
Member Author

@YANG-DB YANG-DB Dec 3, 2024

Choose a reason for hiding this comment

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

@ps48 I'm not sure why this was removed in the main branch
not really involved recently with this repository....

TimeUnit.SECONDS.sleep(1)
}
if (!isIndexExists(INDEX_NAME)) {
val classLoader = ObservabilityIndex::class.java.classLoader
val indexMappingSource = classLoader.getResource(OBSERVABILITY_MAPPING_FILE_NAME)?.readText()!!
val indexSettingsSource = classLoader.getResource(OBSERVABILITY_SETTINGS_FILE_NAME)?.readText()!!
val request = CreateIndexRequest(INDEX_NAME)
.mapping(indexMappingSource, XContentType.YAML)
.settings(indexSettingsSource, XContentType.YAML)
try {
val actionFuture = client.admin().indices().create(request)
val response = actionFuture.actionGet(PluginSettings.operationTimeoutMs)
if (response.isAcknowledged) {
log.info("$LOG_PREFIX:Index $INDEX_NAME creation Acknowledged")
reindexNotebooks()
} else {
error("$LOG_PREFIX:Index $INDEX_NAME creation not Acknowledged")
}
} catch (exception: ResourceAlreadyExistsException) {
log.warn("message: ${exception.message}")
} catch (exception: Exception) {
if (exception.cause !is ResourceAlreadyExistsException) {
throw exception
}
}
this.mappingsUpdated = true
} else if (!this.mappingsUpdated) {
updateMappings()
}
this.mappingsUpdated = true
} else if (!this.mappingsUpdated) {
updateMappings()
} catch (exception: Exception) {
log.error("Unexpected exception while initializing node $exception", exception)
}
}

Expand Down
Loading