Skip to content

fix(graph-dac): push indexed equality filters into the native search query - #1314

Open
likhithThammegowda wants to merge 1 commit into
Sunbird-Knowlg:spark-v1.0.2.1from
Sphere:spark-v1.0.2.1-work
Open

likhithThammegowda wants to merge 1 commit into
Sunbird-Knowlg:spark-v1.0.2.1from
Sphere:spark-v1.0.2.1-work

Conversation

@likhithThammegowda

@likhithThammegowda likhithThammegowda commented Sep 21, 2026 •

Copy link
Copy Markdown

Content create (POST /content/v3/create) blocks for 30 seconds and returns 500, although the node is written in 1.7s. The caller never learns the content was created, so a client retry creates a duplicate.

This PR now contains one commit. The other three it used to carry (99a6a8fd, b35467dd, 2bc0cece) were merged via #1317 on 28 September, so the branch has been rebased onto spark-v1.0.2.1 to leave only what is still new.

Problem

Seen in the creation portal when creating a course, and when saving a resource in the Course Builder: the spinner ran for 30 seconds and failed, but the course had been created, so retrying produced duplicates in My Courses.

expected   POST /content/v3/create  ->  200 in ~3.5s
actual     POST /content/v3/create  ->  500 after 30146 ms   (AskTimeoutException on contentActor)
                                        the node was written to the graph in 1.7s regardless

The 500 is the Pekko ask on contentActor timing out, not the write failing. Reads by identifier are unaffected: they take the extractIdsFromMetadata fast path.

Cause

executeNativeSearch adds IL_SYS_NODE_TYPE and IL_FUNC_OBJECT_TYPE to the JanusGraph query only when the caller sets them via SearchCriteria.setNodeType() / setObjectType(). Callers that carry the same conditions as metadata filters set neither:

  • FrameworkValidator.getMasterCategoryNodes
  • FrameworkValidator.getCategoryTermsFromDB
  • PropAsEdgeValidator

So the base query stays has("graphId", ...) alone. Every node in a graph has the same graphId, so that predicate has no selectivity and the query returns the entire graph. matchesMetadata then filters in memory, and checkFilter → vertex.property() lazy-loads each vertex from the storage backend one round trip at a time.

Isolation: setting master.category.validation.enabled=No, which skips the master-category lookup, took create from a 30146 ms failure to a 3777 ms success. The composite indexes were present and ENABLED throughout; the query simply never used them.

Fix

Indexable equality predicates carried in the metadata are now added to the graph query. Only conditions that must hold for every match are pushed:

  • criteria combined with OR are skipped: an OR branch need not be true, and pushing it would exclude valid matches
  • only OP_EQUAL is pushed: a composite index cannot serve !=, range or list predicates, so status != Retired still applies in memory
  • only keys the graph schema indexes, with a String value

This narrows the candidate set only. The in-memory filter still runs over the result, so the returned nodes are unchanged. The key list is configurable via graph.native_search.indexed_keys; an empty list restores the previous behaviour.

File Change
ontology-engine/graph-dac-api/.../SearchAsyncOperations.java push-down in executeNativeSearch (+60)
ontology-engine/graph-dac-api/.../SearchAsyncOperationsPushDownTest.java new test (161 lines)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Deployed on a Sunbird Spark staging environment, in the knowlg image running there now. POST /content/v3/create returned 200 in 3546 ms with framework validation still enabled, where it previously failed after 30146 ms. The course appeared once, with no duplicate.
  • Unit tests: SearchAsyncOperationsPushDownTest is added and covers the regressed master-category query shape, OR criteria staying in memory, indexed and non-indexed keys filtering identically, both combined, and the unfiltered case. It has not been run locally; please let CI confirm.

The rebased branch was checked against the image deployed on staging: spark-v1.0.2.1 plus this commit produces identical source to what is running there.

Test Configuration:

  • Software versions: Java 11, JanusGraph (Sunbird Spark spark-v1.0.2.1)
  • Hardware versions: Sunbird Spark staging (EKS)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (none needed; the new key has a safe default)
  • My changes generate no new warnings (not checked)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (not run, see above)
  • Any dependent changes have been merged and published in downstream modules (none)

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d74bfb33-4e6f-424d-9daa-85115d3e0e1e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@likhithThammegowda likhithThammegowda changed the title fix(graph): let schema-typed String fields opt out of the JSON-array parse on read fix(graph-dac): push indexed equality filters into the native search query Sep 30, 2026
…query

executeNativeSearch only added IL_SYS_NODE_TYPE and IL_FUNC_OBJECT_TYPE to the
JanusGraph query when the caller had set them via SearchCriteria.setNodeType() /
setObjectType(). Callers that carry the same conditions as metadata filters --
FrameworkValidator.getMasterCategoryNodes and getCategoryTermsFromDB, and
PropAsEdgeValidator -- set neither, so the base query stayed has("graphId", ...)
alone. Every node in a graph carries the same graphId, so that predicate has no
selectivity and the query returned the entire graph. matchesMetadata then filtered
in memory, and checkFilter -> vertex.property() lazy-loaded each vertex from the
storage backend one round trip at a time.

On our staging environment this made content create block until the 30s actor ask
timeout and return 500 -- while the node itself was written in 1.7s, so the content
was created and a client retry duplicated it. Reads by identifier were unaffected:
they take the extractIdsFromMetadata fast path. Setting
master.category.validation.enabled=No, which skips the master-category lookup, took
create from a 30146ms failure to a 3777ms success, isolating this as the cause. The
composite indexes were present and ENABLED throughout; the query simply never used
them.

Indexable equality predicates carried in the metadata are now added to the graph
query. Only conditions that must hold for every match are pushed:

  - criteria combined with OR are skipped, since an OR branch need not be true and
    pushing it would exclude valid matches
  - only OP_EQUAL is pushed; a composite index cannot serve !=, range or list
    predicates, so status != Retired continues to be applied in memory
  - only keys the graph schema indexes, with a String value, are pushed

This narrows the candidate set only -- the in-memory filter still runs over the
result, so the returned nodes are unchanged. The key list is configurable via
graph.native_search.indexed_keys and an empty list restores the previous behaviour.

Tests cover the regressed master-category query shape, OR criteria being left
in memory, indexed and non-indexed keys filtering identically, the two combined,
and the unfiltered case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant