Skip to content

Conversation

@tanik98
Copy link
Contributor

@tanik98 tanik98 commented Jun 20, 2025

Description

  • Adding integration of derived source feature across diff paths #18054 was reverted due to flaky tests with respect to derive source
  • Root cause of the flakyness was due to assertion being done here, in this assertion we are comparing source between two translog index operations(in recovery test case context, one of the operation would be from RecoveryTarget in which order of the document fields might not get preserved)
  • To fix this flakyness, have created TranlogOperationHelper class, in which we are first deriving the source and the comparing the source keeping in mind the order of the source fields.

Failing test example, where order of the doc fields are not the same between two translog index operations:

[2025-06-20T14:15:30,758][INFO ][o.o.i.t.TranslogOperationHelper] [node_s1] op1 source = {name=name_4506, value=4506, timestamp=1750418130310}
[2025-06-20T14:15:30,758][INFO ][o.o.i.t.TranslogOperationHelper] [node_s1] Fetching derived source for 4506
[2025-06-20T14:15:30,773][INFO ][o.o.i.t.TranslogOperationHelper] [node_s1] op1 derive source = {"name":"name_4506","timestamp":"2025-06-20T11:15:30.310Z","value":4506}
[2025-06-20T14:15:30,773][INFO ][o.o.i.t.TranslogOperationHelper] [node_s1] op2 source = {name=name_4506, timestamp=2025-06-20T11:15:30.310Z, value=4506}
[2025-06-20T14:15:30,774][INFO ][o.o.i.t.TranslogOperationHelper] [node_s1] Fetching derived source for 4506
[2025-06-20T14:15:30,775][INFO ][o.o.i.t.TranslogOperationHelper] [node_s1] op2 derive source = {"name":"name_4506","timestamp":"2025-06-20T11:15:30.310Z","value":4506}

Addition of changes as compared to reverted PR

  • Derived source funationality is exactly same as reverted PR
  • Due to need of importing TranslogOperationHelper in TranslogWriter class, we had to flow TranslogOperationHelper starting from the InternalEngine and due to that had to touch these updated files
  • Overriden derived source validation with empty methods for StarTreeMapper, as star-tree field is part of the mapping and we need to skip this field.
  • Falling back to sub keyword field when stored field is not enabled for text field. To pick same sub keyword field each time, we are picking very first sub keyword from lexicographically sorted list.

Related Issues

Resolves #17073

Part of feature #9568

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue has been created.

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.

@github-actions
Copy link
Contributor

✅ Gradle check result for bc53096: SUCCESS

@codecov
Copy link

codecov bot commented Jun 20, 2025

Codecov Report

❌ Patch coverage is 76.79325% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.85%. Comparing base (f0e5003) to head (57b42da).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...rg/opensearch/index/engine/TranslogLeafReader.java 67.85% 11 Missing and 7 partials ⚠️
...search/index/translog/TranslogOperationHelper.java 68.57% 3 Missing and 8 partials ⚠️
...va/org/opensearch/index/engine/InternalEngine.java 72.22% 3 Missing and 2 partials ⚠️
.../main/java/org/opensearch/index/IndexSettings.java 69.23% 3 Missing and 1 partial ⚠️
.../main/java/org/opensearch/index/mapper/Mapper.java 0.00% 2 Missing ⚠️
.../org/opensearch/index/mapper/RootObjectMapper.java 80.00% 2 Missing ⚠️
...va/org/opensearch/index/mapper/StarTreeMapper.java 0.00% 2 Missing ⚠️
...search/index/translog/InternalTranslogFactory.java 0.00% 2 Missing ⚠️
...anslog/RemoteBlobStoreInternalTranslogFactory.java 0.00% 2 Missing ⚠️
...n/java/org/opensearch/index/translog/Translog.java 66.66% 2 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #18565      +/-   ##
============================================
- Coverage     72.88%   72.85%   -0.04%     
- Complexity    69160    69247      +87     
============================================
  Files          5633     5637       +4     
  Lines        317978   318187     +209     
  Branches      45988    46014      +26     
============================================
+ Hits         231752   231808      +56     
- Misses        67517    67660     +143     
- Partials      18709    18719      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added enhancement Enhancement or improvement to existing feature or request Indexing:Performance labels Jun 20, 2025
@tanik98 tanik98 force-pushed the tanik-derived-source-integration branch from f6d0e32 to 62f0be4 Compare June 20, 2025 12:05
@github-actions
Copy link
Contributor

✅ Gradle check result for 62f0be4: SUCCESS

@tanik98 tanik98 changed the title Adding back "Adding integration of derived source feature across diff paths" with fixes for IT Adding back "Adding integration of derived source feature across diff paths" with improvising assertion checks for derived source Jun 20, 2025
@tanik98 tanik98 marked this pull request as ready for review June 20, 2025 13:32
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2025

❌ Gradle check result for 4ba0141: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for f51ffae: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for e2d65dc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for e2d65dc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@mgodwan
Copy link
Member

mgodwan commented Aug 5, 2025

Unexpected exception thrown.
org.gradle.internal.remote.internal.MessageIOException: Could not read message from '/127.0.0.1:40102'.
	at org.gradle.internal.remote.internal.inet.SocketConnection.receive(SocketConnection.java:96)
	at org.gradle.internal.remote.internal.hub.MessageHub$ConnectionReceive.run(MessageHub.java:270)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
	at org.gradle.internal.concurrent.AbstractManagedExecutor$1.run(AbstractManagedExecutor.java:48)
	at java.****/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1095)
	at java.****/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:619)
	at java.****/java.lang.Thread.run(Thread.java:1447)
Caused by: java.lang.IllegalArgumentException
	at org.gradle.internal.remote.internal.hub.InterHubMessageSerializer$MessageReader.read(InterHubMessageSerializer.java:72)
	at org.gradle.internal.remote.internal.hub.InterHubMessageSerializer$MessageReader.read(InterHubMessageSerializer.java:52)
	at org.gradle.internal.remote.internal.inet.SocketConnection.receive(SocketConnection.java:83)
	... 6 more

> Task :plugins:ingest-attachment:test

Doesn't seem related to the PR

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for e2d65dc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: tanik98 <72665765+tanik98@users.noreply.github.com>
@tanik98 tanik98 force-pushed the tanik-derived-source-integration branch from e2d65dc to e4af418 Compare August 5, 2025 10:54
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

✅ Gradle check result for 7df3db6: SUCCESS

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for 35e4ec7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: tanik98 <72665765+tanik98@users.noreply.github.com>
@tanik98 tanik98 force-pushed the tanik-derived-source-integration branch from 35e4ec7 to bbc7c24 Compare August 5, 2025 14:18
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for bbc7c24: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Tanik Pansuriya <panbhai@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

✅ Gradle check result for 59f2bdb: SUCCESS

Signed-off-by: tanik98 <72665765+tanik98@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

❕ Gradle check result for 57b42da: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@Bukhtawar Bukhtawar merged commit cc20ac7 into opensearch-project:main Aug 6, 2025
31 checks passed
vinaykpud pushed a commit to vinaykpud/OpenSearch that referenced this pull request Sep 26, 2025
…et/search/recovery (opensearch-project#18565)

Signed-off-by: Tanik Pansuriya <panbhai@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement or improvement to existing feature or request Indexing:Performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Derived Source] Add support for deriving source field in FieldMapper

4 participants