| From: | lakshmi <lakshmigcdac(at)gmail(dot)com> |
|---|---|
| To: | Filip Janus <fjanus(at)redhat(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, zsolt(dot)parragi(at)percona(dot)com |
| Subject: | Re: Proposal: Adding compression of temporary files |
| Date: | 2026-02-03 17:07:58 |
| Message-ID: | CAEvyyTixmfMhEGrYDqDVOiivazFXFPwARdBhz9qBQQ1fRnYmRQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi all,
I’ve applied the latest two patches on current 19devel and wanted to share
some testing results.
The patches apply cleanly, build and install without issues, and the server
starts normally. I verified that the new temp_file_compression GUC works as
expected and accepts the documented values (no, lz4, pglz), while invalid
values are correctly rejected.
For testing, I forced temp file usage by running parallel hash joins with a
small work_mem. I ran the same query with temp_file_compression set to no,
lz4, and pglz. In all cases, temp files were created and used (hash join
spilling to disk), query results were identical, and I did not see any
crashes or read/write errors.
The temp read/write counters were very similar across all three modes. This
seems expected for hash join spills, since they use fixed-size fileset
chunks, so compression doesn’t necessarily reduce the number of temp blocks
written. Execution time was comparable across modes, with no regressions
observed.
I also ran make check, which passed.
Based on this testing, everything looks good to me, and the observed
behavior matches the documentation clarification about workload-dependent
effects of temp file compression.
Thanks for working on this I’m happy to test further or try additional
workloads if that would be useful.
Best regards,
Lakshmi
On Sun, Jan 25, 2026 at 5:27 PM Filip Janus <fjanus(at)redhat(dot)com> wrote:
> Fixed spacing in the patch.
>
> -Filip-
>
>
> pá 23. 1. 2026 v 17:40 odesílatel Filip Janus <fjanus(at)redhat(dot)com> napsal:
>
>> Hi all,
>> Thanks for the feedback and the provided patch.
>> I've addressed your findings and proposals. Lakshmi's documentation patch
>> was incorporated.
>>
>> -Filip-
>>
>>
>> st 21. 1. 2026 v 7:30 odesílatel lakshmi <lakshmigcdac(at)gmail(dot)com> napsal:
>>
>>> HI all,
>>> While testing the temp file compression patch,noticed that the new
>>> temp_file_compression GUC isn't documented yet.I put together a small docs
>>> patch to add a short description and clarify that the effect of compression
>>> depends on the workload(for example ,hash join spills may not show visible
>>> size reduction due to fixed_size chunks).
>>>
>>> patch is attached.Happy to adjust the wording if needed.
>>> thanks,
>>> lakshmi
>>>
>>> On Tue, Jan 20, 2026 at 4:21 PM lakshmi <lakshmigcdac(at)gmail(dot)com> wrote:
>>>
>>>> Hi Filip,
>>>>
>>>> I tested both patches on current master using git am -3 .They apply
>>>> cleanly,build fine,and the temp_file _compression GUC works as expected.
>>>> Query results are unchanged.
>>>>
>>>> For hash join spill test,temp files were created as expected,but the
>>>> logged size were same for no,lz4,and pglz,which seems consistent with
>>>> fixed-size fileset chunking.It might be helpful to briefly note this in the
>>>> documentation to avoid confusion.
>>>>
>>>> Thanks for working on this .
>>>> best regards,
>>>> lakshmi
>>>>
>>>> On Tue, Jan 20, 2026 at 4:10 AM Zsolt Parragi <
>>>> zsolt(dot)parragi(at)percona(dot)com> wrote:
>>>>
>>>>> Hello!
>>>>>
>>>>> I tried to review the code. It compiled, the test suite passed.
>>>>>
>>>>> I noticed two typos:
>>>>>
>>>>> buffile.c:77 - "Disaled"
>>>>> buffile.c:133 - "mathods"
>>>>>
>>>>> And a few other small findings:
>>>>>
>>>>> buffile.h:35 and buffile.c:63 - same constants defined first as an
>>>>> Enum and then as #defines - code builds properly without the defines.
>>>>>
>>>>> buffile.c:121 - compress_tempfile is defined, set to false at :167,
>>>>> but never used otherwise
>>>>>
>>>>> guc_tables.c:470 - the comment says that pglz isn't supported yet, but
>>>>> we have a value for it, and I see support for it in the code
>>>>>
>>>>> buffile.c:659: (and at other places) if USE_LZ4 is undefined, the
>>>>> codepath doesn't do anything. I think these ifdefs should follow how
>>>>> other compression code works, such as wal compression where there's an
>>>>> #else path with elog(ERROR, ...)
>>>>> Similarly, maybe there should be an explicit TEMP_NONE_COMPRESSION
>>>>> branch that does nothing, and the default branch should be an error?
>>>>>
>>>>> buffile.c:265: If seek isn't supported/limited, shouldn't there be at
>>>>> least an assertion about it in BufFileSeek? And tell isn't mentioned,
>>>>> but it seems to me that tell also doesn't work properly.
>>>>>
>>>>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Greg Sabino Mullane | 2026-02-03 17:08:08 | Re: Pasword expiration warning |
| Previous Message | Greg Sabino Mullane | 2026-02-03 17:03:51 | Re: Change default of jit to off |