Skip to content

perf(scan): skip deleted entries before partition promotion - #3516

Open
anoopj wants to merge 3 commits into
apache:mainfrom
anoopj:followup-3483-manifest-promotion
Open

anoopj wants to merge 3 commits into
apache:mainfrom
anoopj:followup-3483-manifest-promotion

Conversation

@anoopj

@anoopj anoopj commented Oct 23, 2026

Copy link
Copy Markdown
Member

What changes are included in this PR?

Discard deleted manifest entries before copying their metadata or dispatching scan work. Live entries still widen before partition pruning and delete matching, consistent with the spec and Java's live-entry scan.

Share the promotion fixtures' old-schema data manifest and source-drop setup while preserving their schema, format, and sequence-number cases.

Follow-up to #3483.

Are these changes tested?

Covered by existing tests.

Discard deleted manifest entries before copying their metadata or
dispatching scan work. Live entries still widen before partition pruning
and delete matching, consistent with the spec and Java's live-entry scan.

Share the promotion fixtures' old-schema data manifest and source-drop
setup while preserving their schema, format, and sequence-number cases.

Follow-up to apache#3483.

@JeonDaehong JeonDaehong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks right to me. fetch_manifest_and_stream_manifest_entries is the only place that builds a ManifestEntryContext (context.rs:159), so dropping the checks in process_data_manifest_entry / process_delete_manifest_entry loses nothing. Deleted entries also no longer go through promoted_partition, which matches Java planning over live entries only. The red security_audit is RUSTSEC-2026-0385 (#3517), not this change. LGTM.

@laskoviymishka laskoviymishka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice and clean! Thanks!

@laskoviymishka

Copy link
Copy Markdown
Contributor

@anoopj -- will wait for @mbutrovich before merge.

This branch has not been deployed

No deployments
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