Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove IsDeleted filter to improve pinot query performance #6216

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

neil-xie
Copy link
Member

@neil-xie neil-xie commented Aug 7, 2024

What changed?
Detailed Description
Pinot now supports deleteRecordColumn config to filter our deleted records, we no longer need to add the filter in our queries.

Impact Analysis

  • Backward Compatibility: No impact, config already changed in internal Pinot table
  • Forward Compatibility: No impact

Why?
Pinot table supports filtering deleted records

How did you test it?
Testing Plan

  • Unit Tests: Existing test
  • Persistence Tests: Not related
  • Integration Tests: Existing test
  • Compatibility Tests: Not related

Rollout Plan

  • What is the rollout plan? Normal rollout
  • Does the order of deployment matter? No
  • Is it safe to rollback? Does the order of rollback matter? Yes
  • Is there a kill switch to mitigate the impact immediately? Not needed

Potential risks

Release notes

Documentation Changes

Copy link
Member

@bowenxia bowenxia left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.93%. Comparing base (0409857) to head (b3419cc).

Additional details and impacted files
Files Coverage Δ
common/persistence/pinot/pinot_visibility_store.go 93.51% <ø> (-0.08%) ⬇️
...rker/esanalyzer/domainWorkflowTypeCountWorkflow.go 83.13% <ø> (-0.11%) ⬇️

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0409857...b3419cc. Read the comment docs.

@neil-xie neil-xie merged commit d5814da into cadence-workflow:master Aug 8, 2024
20 checks passed
@neil-xie neil-xie deleted the cdnc_10028 branch August 8, 2024 14:11
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.

3 participants