Skip to content

Have fsTablePersister implement movingTableFilePersister#10691

Merged
nicktobey merged 1 commit intomainfrom
nicktobey/movingTableFilePersister
Mar 16, 2026
Merged

Have fsTablePersister implement movingTableFilePersister#10691
nicktobey merged 1 commit intomainfrom
nicktobey/movingTableFilePersister

Conversation

@nicktobey
Copy link
Copy Markdown
Contributor

The movingTableFilePersister interface is for tablePersister implementations that build a temporary table file and have the ability to move the temporary file into the final location instead of copying the contents into a new one.

#9118 added support for persisting archive files. As part of this change, the signature of a method on movingTableFilePersister was changed to accept the more generic GenericTableWriter interface instead of the more specific CmpChunkTableWriter.

Unfortunately, the signature was not also updated on the fsTablePersister implementation, resulting in it no longer implementing the interface. As a result, we lost the ability move table files during garbage collection. This had no impact on correctness, but potentially had a performance impact.

This PR fixes this issue.

Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

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

LGTM

@coffeegoddd
Copy link
Copy Markdown
Contributor

@nicktobey DOLT

read_tests from_latency to_latency percent_change
covering_index_scan 0.55 0.56 1.82
groupby_scan 9.91 10.09 1.82
index_join 1.86 1.82 -2.15
index_join_scan 1.39 1.34 -3.6
index_scan 21.89 21.89 0.0
oltp_point_select 0.27 0.27 0.0
oltp_read_only 5.28 5.28 0.0
select_random_points 0.53 0.53 0.0
select_random_ranges 0.55 0.55 0.0
table_scan 21.89 22.28 1.78
types_table_scan 66.84 66.84 0.0
write_tests from_latency to_latency percent_change
oltp_delete_insert 6.32 6.32 0.0
oltp_insert 3.13 3.13 0.0
oltp_read_write 11.24 11.24 0.0
oltp_update_index 3.19 3.19 0.0
oltp_update_non_index 3.13 3.13 0.0
oltp_write_only 5.99 5.99 0.0
types_delete_insert 6.91 6.91 0.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000
version result total
a7b1286 ok 5937471
version total_tests
a7b1286 5937471
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

@nicktobey DOLT

test_name from_latency_p95 to_latency_p95 percent_change
tpcc-scale-factor-1 61.08 61.08 0.0
test_name from_server_name from_server_version from_tps to_server_name to_server_version to_tps percent_change
tpcc-scale-factor-1 dolt 1bd70a3 37.49 dolt a7b1286 37.7 0.56

@nicktobey nicktobey merged commit ff749ef into main Mar 16, 2026
45 of 49 checks passed
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.

3 participants