Skip to content

Translate processor performance improvements#6093

Merged
san81 merged 9 commits into
opensearch-project:mainfrom
san81:translate-processor-improvements
Sep 23, 2025
Merged

Translate processor performance improvements#6093
san81 merged 9 commits into
opensearch-project:mainfrom
san81:translate-processor-improvements

Conversation

@san81

@san81 san81 commented Sep 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

Continuing the work of using EventKey in the place of String based keys #6020

Even though, PR looks huge, majority of them are just adding json files for testing scenarios. Primary code changes are just in TranslateProcessor EventKey caching. Rest of the changes are adding additional test coverage.

Issues Resolved

#6020

Due to the caching of the event, below JMH bechmark numbers may be really helpful to understand the impact but just keeping them here as an FYI.

jmh benchmark before this change


Benchmark                                                       Mode  Cnt          Score         Error  Units
TranslateProcessorBenchmark.benchmark_dynamic_key_translation  thrpt    6  124285135.602 ± 1418774.504  ops/s
TranslateProcessorBenchmark.benchmark_nested_path_translation  thrpt    6  275961895.249 ± 4261128.734  ops/s
TranslateProcessorBenchmark.benchmark_pattern_matching         thrpt    6   79327143.311 ± 1396818.087  ops/s
TranslateProcessorBenchmark.benchmark_static_key_translation   thrpt    6  124282297.525 ± 1948324.949  ops/s

jmh benchmark after this change




Benchmark                                                       Mode  Cnt           Score           Error  Units
TranslateProcessorBenchmark.benchmark_dynamic_key_translation  thrpt   10  1556663215.732 ±  15425311.375  ops/s
TranslateProcessorBenchmark.benchmark_nested_path_translation  thrpt   10  1535981591.007 ±  46548904.997  ops/s
TranslateProcessorBenchmark.benchmark_pattern_matching         thrpt   10  1505730352.975 ± 146716619.161  ops/s
TranslateProcessorBenchmark.benchmark_static_key_translation   thrpt   10  1550222201.638 ±  38707357.383  ops/s

Percentage change in each cateogry

Benchmark                                                       %change  
TranslateProcessorBenchmark.benchmark_dynamic_key_translation 1152%
TranslateProcessorBenchmark.benchmark_nested_path_translation  456%
TranslateProcessorBenchmark.benchmark_pattern_matching         1798%
TranslateProcessorBenchmark.benchmark_static_key_translation   1147%

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

@san81
minor performance fix and additional tests added
4b31a23 
Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
@san81
jmh benchmark added to Translate processor
3563c06 
Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
@san81
String based event key is now replaced with EventKey
50a50e2 
Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
@san81
fixed failing tests
d4830e0 
Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
@san81
Merge branch 'opensearch-project:main' into translate-processor-impro…
aaf2edf 
…vements
@san81
increasing the jmh iteration count
01a9c91 
Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
@san81
Merge branch 'opensearch-project:main' into translate-processor-impro…
77aeb64 
…vements
dlvenable
dlvenable reviewed Sep 22, 2025
View reviewed changes
Comment thread ...src/main/java/org/opensearch/dataprepper/plugins/processor/translate/CachingKeyResolver.java
/**
* KeyResolver implementation with ConcurrentHashMap-based caching.
*/
class CachingKeyResolver implements KeyResolver {

@dlvenable dlvenable Sep 22, 2025

Copy link
Copy Markdown
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 this? The EventKeyFactory already has a cache.

@san81 san81 Sep 23, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is caching the entire KeyInfo instances, that includes additional flags on the TranslateProcessor mapped keys. Otherwise, this logic will go into the Translate processor itself. That is another option. This is just to segregate that logic outside of that class.

@dlvenable dlvenable Sep 23, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this also includes some of the expression parsing I see. Thanks!

Comment thread ...src/main/java/org/opensearch/dataprepper/plugins/processor/translate/TranslateProcessor.java Outdated
@DataPrepperPluginConstructor
public TranslateProcessor(PluginMetrics pluginMetrics, final TranslateProcessorConfig translateProcessorConfig, final ExpressionEvaluator expressionEvaluator) {
public TranslateProcessor(
PluginMetrics pluginMetrics,

@dlvenable dlvenable Sep 22, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add the final modifier.

@san81 san81 Sep 23, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added 👍

@san81
Addressing review comments
ba28712 
Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
dlvenable
dlvenable previously approved these changes Sep 23, 2025
View reviewed changes
sb2k16
sb2k16 previously approved these changes Sep 23, 2025
View reviewed changes
@san81
Keeping previous mock based test class as it is
a957229 
Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
@san81 san81 dismissed stale reviews from sb2k16 and dlvenable via a957229 September 23, 2025 16:23
@san81 san81 merged commit 3438936 into opensearch-project:main Sep 23, 2025
45 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dlvenable dlvenable dlvenable approved these changes
@sb2k16 sb2k16 sb2k16 approved these changes
@chenqi0805 chenqi0805 Awaiting requested review from chenqi0805
@engechas engechas Awaiting requested review from engechas
@srikanthjg srikanthjg Awaiting requested review from srikanthjg srikanthjg is a code owner
@graytaylor0 graytaylor0 Awaiting requested review from graytaylor0 graytaylor0 is a code owner
@dinujoh dinujoh Awaiting requested review from dinujoh dinujoh is a code owner
@kkondaka kkondaka Awaiting requested review from kkondaka kkondaka is a code owner
@KarstenSchnitter KarstenSchnitter Awaiting requested review from KarstenSchnitter KarstenSchnitter is a code owner
@oeyh oeyh Awaiting requested review from oeyh oeyh is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@san81 @dlvenable @sb2k16