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

use inserts instead of lightweight deletes to remove reorged data #135

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iuwqyir
Copy link
Collaborator

@iuwqyir iuwqyir commented Jan 16, 2025

TL;DR

Replaced DELETE operations with INSERT operations using is_deleted flag in Clickhouse tables for better performance and eventual data consistency.

What changed?

  • Replaced direct DELETE queries with INSERT operations setting is_deleted=1
  • Removed lightweight_deletes_sync setting from Clickhouse configuration
  • Simplified function names by removing "ByNumbers" suffix

How to test?

  1. Delete block data using the DeleteBlockData function
  2. Verify that records are marked as deleted (is_deleted=1) instead of being physically removed
  3. Confirm that queries properly filter out deleted records
  4. Check that deletion operations complete successfully for blocks, logs, transactions, and traces

Why make this change?

  • Improves deletion performance by avoiding expensive DELETE operations
  • Provides more consistent and reliable deletion operations across all tables

Copy link
Collaborator Author

iuwqyir commented Jan 16, 2025

@iuwqyir iuwqyir requested a review from a team January 16, 2025 00:05
@iuwqyir iuwqyir marked this pull request as ready for review January 16, 2025 00:06
@iuwqyir iuwqyir force-pushed the 01-16-use_inserts_instead_of_lightweight_deletes_to_remove_reorged_data branch from 74accd7 to 20860dc Compare January 16, 2025 15:05
@iuwqyir iuwqyir force-pushed the 01-16-use_inserts_instead_of_lightweight_deletes_to_remove_reorged_data branch from 20860dc to 642b121 Compare January 16, 2025 15:07
@@ -173,6 +173,8 @@ var allowedFunctions = map[string]struct{}{
"toStartOfDay": {},
"toDate": {},
"concat": {},
"in": {},
"IN": {},
Copy link
Contributor

Choose a reason for hiding this comment

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

why adding both cases? maybe then it's better to change the logic to actually always lowercase?

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.

2 participants