Improve logging by reducing noise and logging high-level events#350
Open
sfc-gh-mslot wants to merge 5 commits into
Open
Improve logging by reducing noise and logging high-level events#350sfc-gh-mslot wants to merge 5 commits into
sfc-gh-mslot wants to merge 5 commits into
Conversation
Signed-off-by: Marco Slot <marco.slot@snowflake.com>
Log a one-line summary after each cache-pressure pass (files/bytes added, evicted, and skipped by reason) so we can see why the cache stayed full or churned. Append the first 500 chars of the failing query to pgduck_server parse/bind/execute WARNING lines so a stray error is debuggable without replaying the session. Signed-off-by: Marco Slot <marco.slot@snowflake.com>
Collaborator
sfc-gh-okalaci
left a comment
sfc-gh-okalaci
left a comment
There was a problem hiding this comment.
Well, you can see all my notes as optional.
I'm a bit torn on the user-initiated lines. The user already sees those, they ran the command and got the result back over the wire, and a DBA who wants an audit trail can use log_statement = ddl.
The background ones are exactly the right thing to log at LOG though.
+1 on demoting the per-write adding/removing/expiring noise.
Wrap the remote OpenFile call and both Write overloads with a try block that emits a PGDUCK_SERVER_WARN for ExceptionType::HTTP and rethrows. DuckDB's HTTPException message already carries URL, status code, and reason, so a 404, 403, or 500 on an S3 object now surfaces a single WARNING line before the error propagates to the client. Non-HTTP exceptions pass through silently. Signed-off-by: Marco Slot <marco.slot@snowflake.com>
sfc-gh-mslot
force-pushed
the
marcoslot/improve-logging
branch
from
dd6013d to
c2a59cb
Compare
sfc-gh-mslot
added a commit
that referenced
this pull request
May 15, 2026
- Demote "adding %s to cache" to DEBUG1 to match the existing "removing" counterpart (#350: Onur). - Drop the LOG line for CREATE TABLE / DROP TABLE; per Onur, these events aren't valuable to log unconditionally. The compaction log in VACUUM (which runs in the background) stays. - Drop the LOG line for COPY TO; same reason. Removes the now-unused StatsCollector capture and data_file_stats.h include. - Refactor compaction stat bumps in TryCompactDataFiles into RecordRemovedFile / RecordAddedFile helpers (the runStats != NULL guard moves into the helper). - Add test_vacuum_compaction_summary_log to verify the VACUUM "compacted iceberg table" LOG line emits exactly once with the expected file/byte/row counts.
- Demote "adding %s to cache" to DEBUG1 to match the existing "removing" counterpart (#350: Onur). - Drop the LOG line for CREATE TABLE / DROP TABLE; per Onur, these events aren't valuable to log unconditionally. The compaction log in VACUUM (which runs in the background) stays. - Drop the LOG line for COPY TO; same reason. Removes the now-unused StatsCollector capture and data_file_stats.h include. - Refactor compaction stat bumps in TryCompactDataFiles into RecordRemovedFile / RecordAddedFile helpers (the runStats != NULL guard moves into the helper). - Add test_vacuum_compaction_summary_log to verify the VACUUM "compacted iceberg table" LOG line emits exactly once with the expected file/byte/row counts. Signed-off-by: Marco Slot <marco.slot@snowflake.com>
sfc-gh-mslot
force-pushed
the
marcoslot/improve-logging
branch
from
952c0ab to
c2a59cb
Compare
The setup path waits 0.1s after pg_reload_conf so backends pick up ALTER SYSTEM SET before the test runs. The teardown path skipped the sleep, so the next test could see still-applied ALTER SYSTEM values on its module-scoped pg_conn. Mirror the setup so the RESET propagates before the next test starts. Surfaced by pytest-split rebalance after adding test_vacuum_compaction_summary_log: shard 3 sometimes ran test_writable_object_store_catalog_options immediately before test_writable_object_store_without_default_location_guc, where the leftover object_store_catalog_location_prefix flipped which error path fired. Signed-off-by: Marco Slot <marco.slot@snowflake.com>
sfc-gh-mslot
force-pushed
the
marcoslot/improve-logging
branch
from
08245c4 to
803c555
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduce noise and have more meaningful messages like: