Skip to content

Conversation

Tishj
Copy link
Contributor

@Tishj Tishj commented Sep 10, 2024

This PR fixes https://github.com/duckdblabs/duckdb-internal/issues/2805

evicted_data_per_tag gets increased in WriteTemporaryBuffer, but gets decreased in ReadTemporaryBuffer.
This meant that evicted_data_per_tag would never decrease if the temporary buffer was not read before it was destroyed.

This PR moves this logic to DeleteTemporaryFile which will happen both when the buffer is read (as it gets subsequently destroyed) and also when it is destroyed before reading.

…n Read, but rather on Delete. If the data is not read before it is deleted the evicted data would not decrease properly
@duckdb-draftbot duckdb-draftbot marked this pull request as draft September 11, 2024 09:05
@Tishj Tishj marked this pull request as ready for review September 11, 2024 09:21
@Mytherin Mytherin merged commit c41ae2c into duckdb:main Sep 11, 2024
41 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Sep 25, 2024
[BufferManager] Fix `duckdb_memory()` reporting wrong size for `temporary_storage_bytes` (duckdb/duckdb#13837)
github-actions bot added a commit to duckdb/duckdb-r that referenced this pull request Sep 25, 2024
[BufferManager] Fix `duckdb_memory()` reporting wrong size for `temporary_storage_bytes` (duckdb/duckdb#13837)

Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
@alfeg
Copy link

alfeg commented Jun 23, 2025

Not sure if it's proper place to report.

But it seems that temporary_storage_bytes is still reporting wrong values
image
This a screenshot of our internal app reporting. It's executed duckdb_memory() and shows as table.
Server where this application is running have only 64Gb of disk space.
Actual temp folder is actually empty on the moment of the screen capture.

I cannot provide a repro as it's very data specific.

Any existing reporting on this?

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