Add EngineConfig extensions to EnginePlugin #1387
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds an extension point to EngineConfig through EnginePlugin using a new EngineConfigFactory mechanism. EnginePlugin provides interface methods to override configurations in EngineConfig. The EngineConfigFactory produces a new instance of the EngineConfig using these overrides. Defaults are used absent overridden configurations. This serves as a mechanism to override TranslogConfig and CodecService enabling Plugins to have higher fidelity for changing Engine configurations without having to override the entire Engine. Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
nknize
added
enhancement
|
Can one of the admins verify this patch? |
|
✅ Gradle Wrapper Validation success ee4aaf8 |
|
✅ DCO Check Passed ee4aaf8 |
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
|
✅ Gradle Wrapper Validation success df28fdf |
|
✅ DCO Check Passed df28fdf |
|
✅ Gradle Precommit success df28fdf |
reta
reviewed
Oct 19, 2021
server/src/main/java/org/opensearch/index/shard/IndexShard.java
Outdated
Show resolved
Hide resolved
reta
reviewed
Oct 19, 2021
test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java
Show resolved
Hide resolved
|
✅ DCO Check Passed 767847e82cd12645ce6e60d70c24ea2090a56906 |
|
✅ Gradle Wrapper Validation success 767847e82cd12645ce6e60d70c24ea2090a56906 |
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
nknize
force-pushed
the
enginePlugin/EngineConfigFactory
branch
from
767847e to
a116d3a
Compare
|
✅ DCO Check Passed a116d3a |
|
✅ Gradle Wrapper Validation success a116d3a |
reta
approved these changes
Oct 19, 2021
|
❌ Gradle Precommit failure 767847e82cd12645ce6e60d70c24ea2090a56906 |
|
✅ Gradle Precommit success a116d3a |
|
start gradle check |
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
|
start gradle check |
|
✅ Gradle Wrapper Validation success 6394df6 |
|
✅ DCO Check Passed 6394df6 |
|
✅ Gradle Precommit success 6394df6 |
setiah
reviewed
Oct 19, 2021
server/src/main/java/org/opensearch/index/engine/EngineConfigFactory.java
Show resolved
Hide resolved
setiah
approved these changes
Oct 19, 2021
adnapibar
approved these changes
Oct 20, 2021
| * A factory to create an EngineConfig based on custom plugin overrides | ||
| */ | ||
| public class EngineConfigFactory { | ||
| private final Optional<CodecService> codecService; |
There was a problem hiding this comment.
This doesn't need to be declared with Optional
There was a problem hiding this comment.
It was mostly for the logic on L111.
this.codecService.isPresent() ? this.codecService.get() : codecService;
instead of
this.codecService != null ? this.codecService : codecService;
Which feels a little clumsy?
nknize
added
the
pending backport
nknize
added a commit
to nknize/OpenSearch
that referenced
this pull request
Oct 21, 2021
This commit adds an extension point to EngineConfig through EnginePlugin using a new EngineConfigFactory mechanism. EnginePlugin provides interface methods to override configurations in EngineConfig. The EngineConfigFactory produces a new instance of the EngineConfig using these overrides. Defaults are used absent overridden configurations. This serves as a mechanism to override Engine configurations (e.g., CodecService, TranslogConfig) enabling Plugins to have higher fidelity for changing Engine behavior without having to override the entire Engine (which is only permitted for a single plugin). Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
nknize
added a commit
that referenced
this pull request
Oct 21, 2021
This commit adds an extension point to EngineConfig through EnginePlugin using a new EngineConfigFactory mechanism. EnginePlugin provides interface methods to override configurations in EngineConfig. The EngineConfigFactory produces a new instance of the EngineConfig using these overrides. Defaults are used absent overridden configurations. This serves as a mechanism to override Engine configurations (e.g., CodecService, TranslogConfig) enabling Plugins to have higher fidelity for changing Engine behavior without having to override the entire Engine (which is only permitted for a single plugin). Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
|
Does this still need to be backported? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an extension point to
EngineConfigthroughEnginePluginusinga new
EngineConfigFactorymechanism.EnginePluginprovides interface methods tooverride configurations in
EngineConfig. TheEngineConfigFactoryproduces a newinstance of the
EngineConfigusing these overrides. Defaults are used absentoverridden configurations.
This serves as a mechanism to override Engine configurations (e.g.,
CodecService,TranslogConfig) enabling Plugins to have higher fidelity for changing Enginebehavior without having to override the entire
Engine(which is only permitted fora single plugin).