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

Support for system index interface #789

Merged
merged 8 commits into from
May 30, 2023
+31 −3
Merged

Support for system index interface #789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c22cff7
Support for system index interface
bowenlan-amzn May 24, 2023
a7b15e3
Handle rejected execution exception as retryable in retry block
bowenlan-amzn May 24, 2023
d1ac84f
Fix tests
bowenlan-amzn May 24, 2023
af8a5f9
Merge branch '2.x' into 2.x
bowenlan-amzn May 25, 2023
cd842e3
Fix tests
bowenlan-amzn May 26, 2023
a127af7
Merge branch '2.x' of github.com:bowenlan-amzn/index-management into 2.x
bowenlan-amzn May 26, 2023
b4c4461
Merge branch '2.x' into 2.x
bowenlan-amzn May 26, 2023
6dbf2ab
Merge branch '2.x' into 2.x
bowenlan-amzn May 30, 2023
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
8 changes: 7 additions & 1 deletion src/main/kotlin/org/opensearch/indexmanagement/IndexManagementPlugin.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ import org.opensearch.indexmanagement.transform.resthandler.RestPreviewTransform
import org.opensearch.indexmanagement.transform.resthandler.RestStartTransformAction
import org.opensearch.indexmanagement.transform.resthandler.RestStopTransformAction
import org.opensearch.indexmanagement.transform.settings.TransformSettings
import org.opensearch.indices.SystemIndexDescriptor
import org.opensearch.jobscheduler.spi.JobSchedulerExtension
import org.opensearch.jobscheduler.spi.ScheduledJobParser
import org.opensearch.jobscheduler.spi.ScheduledJobRunner
Expand All @@ -185,6 +186,7 @@ import org.opensearch.plugins.ActionPlugin
import org.opensearch.plugins.ExtensiblePlugin
import org.opensearch.plugins.NetworkPlugin
import org.opensearch.plugins.Plugin
import org.opensearch.plugins.SystemIndexPlugin
import org.opensearch.repositories.RepositoriesService
import org.opensearch.rest.RestController
import org.opensearch.rest.RestHandler
Expand All @@ -197,7 +199,7 @@ import org.opensearch.watcher.ResourceWatcherService
import java.util.function.Supplier

@Suppress("TooManyFunctions")
class IndexManagementPlugin : JobSchedulerExtension, NetworkPlugin, ActionPlugin, ExtensiblePlugin, Plugin() {
class IndexManagementPlugin : JobSchedulerExtension, NetworkPlugin, ActionPlugin, ExtensiblePlugin, SystemIndexPlugin, Plugin() {

private val logger = LogManager.getLogger(javaClass)
lateinit var indexManagementIndices: IndexManagementIndices
Expand Down Expand Up @@ -613,6 +615,10 @@ class IndexManagementPlugin : JobSchedulerExtension, NetworkPlugin, ActionPlugin
override fun getActionFilters(): List<ActionFilter> {
return listOf(fieldCapsFilter, indexOperationActionFilter)
}

override fun getSystemIndexDescriptors(settings: Settings): Collection<SystemIndexDescriptor> {
return listOf(SystemIndexDescriptor(INDEX_MANAGEMENT_INDEX, "Index for storing index management configuration and metadata."))
}
}

class GuiceHolder @Inject constructor(
Expand Down
7 changes: 7 additions & 0 deletions src/main/kotlin/org/opensearch/indexmanagement/opensearchapi/OpenSearchExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import org.opensearch.common.xcontent.XContentType
import org.opensearch.commons.InjectSecurity
import org.opensearch.commons.authuser.User
import org.opensearch.commons.notifications.NotificationsPluginInterface
import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException
import org.opensearch.core.xcontent.MediaType
import org.opensearch.core.xcontent.NamedXContentRegistry
import org.opensearch.core.xcontent.ToXContent
Expand Down Expand Up @@ -181,6 +182,12 @@ suspend fun <T> BackoffPolicy.retry(
} else {
throw e
}
} catch (rje: OpenSearchRejectedExecutionException) {
if (iter.hasNext()) {
Hailong-am marked this conversation as resolved.
Show resolved Hide resolved
backoff = iter.next()
logger.warn("Rejected execution. Retrying in $backoff.", rje)
delay((backoff.millis))
Comment on lines +187 to +189
Copy link
Member

Choose a reason for hiding this comment

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

Shall we add test to verify the behavior for this exception handling?

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't feel there's need to check such simple logic 😛 no test for the existing block also

Copy link
Member

Choose a reason for hiding this comment

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

Its good to have unit tests for code coverage and setting the code expectation for future (or accidental) changes.

}
}
} while (true)
}
Expand Down
9 changes: 7 additions & 2 deletions src/test/kotlin/org/opensearch/indexmanagement/IndexManagementRestTestCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ abstract class IndexManagementRestTestCase : ODFERestTestCase() {
// preemptively seems to give the job scheduler time to listen to operations.
@Before
fun initializeManagedIndex() {
if (!indexExists(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX)) {
if (!isIndexExists(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX)) {
val request = Request("PUT", "/${IndexManagementPlugin.INDEX_MANAGEMENT_INDEX}")
var entity = "{\"settings\": " + Strings.toString(XContentType.JSON, Settings.builder().put(INDEX_HIDDEN, true).build())
entity += ",\"mappings\" : ${IndexManagementIndices.indexManagementMappings}}"
Expand All @@ -77,6 +77,11 @@ abstract class IndexManagementRestTestCase : ODFERestTestCase() {

protected fun Response.restStatus(): RestStatus = RestStatus.fromCode(this.statusLine.statusCode)

protected fun isIndexExists(index: String): Boolean {
Copy link
Member

Choose a reason for hiding this comment

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

Can we also assert on System Index property here to ensure it always remains a system index?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a REST API call.
It seems there's no way to check whether an index is system index using REST API.
https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/indices/SystemIndices.java

val response = client().makeRequest("HEAD", index)
return RestStatus.OK == response.restStatus()
}

protected fun assertIndexExists(index: String) {
val response = client().makeRequest("HEAD", index)
assertEquals("Index $index does not exist.", RestStatus.OK, response.restStatus())
Expand Down Expand Up @@ -165,7 +170,7 @@ abstract class IndexManagementRestTestCase : ODFERestTestCase() {
override fun preserveIndicesUponCompletion(): Boolean = true
companion object {
@JvmStatic
protected val isMultiNode = System.getProperty("cluster.number_of_nodes", "1").toInt() > 1
val isMultiNode = System.getProperty("cluster.number_of_nodes", "1").toInt() > 1
protected val defaultKeepIndexSet = setOf(".opendistro_security")
/**
* We override preserveIndicesUponCompletion to true and use this function to clean up indices
Expand Down
6 changes: 6 additions & 0 deletions src/test/kotlin/org/opensearch/indexmanagement/TestHelpers.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import org.opensearch.client.Response
import org.opensearch.client.RestClient
import org.opensearch.client.WarningsHandler
import org.opensearch.commons.authuser.User
import org.opensearch.indexmanagement.IndexManagementRestTestCase.Companion.isMultiNode
import org.opensearch.jobscheduler.spi.schedule.CronSchedule
import org.opensearch.jobscheduler.spi.schedule.IntervalSchedule
import org.opensearch.jobscheduler.spi.schedule.Schedule
Expand Down Expand Up @@ -113,6 +114,11 @@ fun <T> waitFor(
timeout: Instant = Instant.ofEpochSecond(20),
block: () -> T
): T {
if (isMultiNode) {
// job scheduling could be skipped in multi-node tests
// https://github.com/opensearch-project/job-scheduler/issues/173
timeout.plusSeconds(60)
}
val startTime = Instant.now().toEpochMilli()
do {
try {
Expand Down
2 changes: 2 additions & 0 deletions src/test/kotlin/org/opensearch/indexmanagement/rollup/resthandler/RestStartRollupActionIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ class RestStartRollupActionIT : RollupRestTestCase() {
assertEquals("Rollup is not RETRY", RollupMetadata.Status.RETRY, rollupMetadata.status)

// clearing the config index to prevent other tests using this multi shard index
Thread.sleep(2000L)
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to wait before deletion. Also adding sleeps in general tend to make tests flaky.

Copy link
Member Author

Choose a reason for hiding this comment

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

Before it's to wait for rollup metadata update to finish, observe the metadata update may recreate the index after deletion. But cannot guarantee it's the root cause.
This flaky is not related to the change in this PR.

deleteIndex(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX)
Thread.sleep(2000L)
}
}
2 changes: 2 additions & 0 deletions src/test/kotlin/org/opensearch/indexmanagement/rollup/resthandler/RestStopRollupActionIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ class RestStopRollupActionIT : RollupRestTestCase() {
assertEquals("Rollup is not STOPPED", RollupMetadata.Status.STOPPED, rollupMetadata.status)

// clearing the config index to prevent other tests using this multi shard index
Thread.sleep(2000L)
deleteIndex(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX)
Thread.sleep(2000L)
}
}