Skip to content

[SDBM-1807] Do not submit plan events when plan definition has been evicted from cache - #20584

Merged
natashadada merged 7 commits into
masterfrom
natasha.dada/SDBM-1807
Jun 30, 2025
Merged

natashadada merged 7 commits into
masterfrom
natasha.dada/SDBM-1807

Conversation

@natashadada

@natashadada natashadada commented Jun 24, 2025 •

Copy link
Copy Markdown

There are cases where we are not able get a plan from the cache (ex. sqlserver evicted the relevant plan to free up memory). When this happens, we still submit a plan event with a plan signature but no plan definition. When querying for plans this is a bit weird since we say we've collected a plan because we see a plan event with a plan signature but we don't have the actual plan to show the user.

I see a few options here:

  1. We can not submit plan events if we don't have the plan definition. This makes the most sense to me right now but would appreciate other perspectives/things I may be missing
  2. We could submit the plan with a collection_error that says the plan is null because it was not found
  3. We could leave plan collection as is and change the way we query for plans on the front end (only query for plan events with a plan definition)

This PR is for option 1. If there is a collection errors, then we still submit the plan but if there's not collection error and no plan definition, we skip the plan event. We have a log line when we fail to load plan, so users can still see when this happens and we can see it in debug flares.

If a plan is encrypted, we don't get a plan definition. In this case, I am adding a collection error. I will update the front end to have support for this error in https://github.com/DataDog/web-ui/pull/212997.

Support ticket: https://datadoghq.atlassian.net/browse/SDBM-1807

@natashadada
natashadada force-pushed the natasha.dada/SDBM-1807 branch from 0ccce83 to a09cfe0 Compare June 25, 2025 17:34
@codecov

codecov Bot commented Jun 25, 2025 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.38%. Comparing base (077bcac) to head (42def46).
⚠️ Report is 1557 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
activemq ?
cassandra ?
confluent_platform ?
hive ?
hivemq ?
hudi ?
ignite ?
jboss_wildfly ?
kafka ?
presto ?
solr ?
sqlserver 91.27% <100.00%> (+5.30%) ⬆️
tomcat ?
weblogic ?

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@natashadada natashadada changed the title [SDBM-1807] Only submit plan events with plan definitions [SDBM-1807] Do not submit plan events when plan definition has been evicted from cache Jun 26, 2025
@natashadada
natashadada added this pull request to the merge queue Jun 30, 2025
Merged via the queue into master with commit 80c1637 Jun 30, 2025
@natashadada
natashadada deleted the natasha.dada/SDBM-1807 branch June 30, 2025 14:54
github-actions Bot pushed a commit that referenced this pull request Jun 30, 2025
…victed from cache (#20584)

* [SDBM-1807] Only submit plan events with plan definitions

* lint

* fix

* lint

* lint

* fix

* update test 80c1637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants