Skip to content

Fix wrong merged table check in JFRDataQueryEsDAO#13805

Merged
mrproliu merged 2 commits intoapache:masterfrom
currenjin:fix/es-jfr-wrong-merged-table-check
Apr 8, 2026
Merged

Fix wrong merged table check in JFRDataQueryEsDAO#13805
mrproliu merged 2 commits intoapache:masterfrom
currenjin:fix/es-jfr-wrong-merged-table-check

Conversation

@currenjin
Copy link
Copy Markdown
Contributor

@currenjin currenjin commented Apr 8, 2026

What's this PR about?

JFRDataQueryEsDAO.getByTaskIdAndInstancesAndEvent() passes the wrong index name to isMergedTable().

// before
if (IndexController.LogicIndicesRegister.isMergedTable(AsyncProfilerTaskRecord.INDEX_NAME)) {
// after
if (IndexController.LogicIndicesRegister.isMergedTable(JFRProfilingDataRecord.INDEX_NAME)) {

The method queries jfr_profiling_data, but the merged table check uses async_profiler_task. Every other ES DAO passes its own INDEX_NAME. This one was copied from AsyncProfilerTaskQueryEsDAO without updating.

With the default config (logicSharding=false), both tables resolve to records-all, so isMergedTable() returns true for both and the behavior happens to be correct. But with logicSharding=true, the two tables can have different physical names, and the check would produce the wrong result.

Which issue does this PR relate to?

N/A - found by code review.

@wu-sheng wu-sheng requested review from Copilot and mrproliu April 8, 2026 07:55
@wu-sheng wu-sheng added this to the 10.5.0 milestone Apr 8, 2026
@wu-sheng wu-sheng added the bug Something isn't working and you are sure it's a bug! label Apr 8, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an Elasticsearch DAO query guard so JFRDataQueryEsDAO.getByTaskIdAndInstancesAndEvent() correctly checks whether the JFR profiling data index is configured as a merged table, ensuring the merged-table discriminator filter is applied (or skipped) against the right logical index.

Changes:

  • Switch isMergedTable() to use JFRProfilingDataRecord.INDEX_NAME instead of AsyncProfilerTaskRecord.INDEX_NAME.
  • Remove the now-unused AsyncProfilerTaskRecord import.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mrproliu mrproliu merged commit cb742a2 into apache:master Apr 8, 2026
417 of 420 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working and you are sure it's a bug!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants