From: | Noah Misch <noah(at)leadboat(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | mdclose() does not cope w/ FileClose() failure |
Date: | 2019-12-22 09:19:30 |
Message-ID: | 20191222091930.GA1280238@rfd.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Forking thread "WAL logging problem in 9.4.3?" for this tangent:
On Mon, Dec 09, 2019 at 06:04:06PM +0900, Kyotaro Horiguchi wrote:
> I don't understand why mdclose checks for (v->mdfd_vfd >= 0) of open
> segment but anyway mdimmedsync is believing that that won't happen and
> I follow the assumption. (I suspect that the if condition in mdclose
> should be an assertion..)
That check helps when data_sync_retry=on and FileClose() raised an error in a
previous mdclose() invocation. However, the check is not sufficient to make
that case work; the attached test case (not for commit) gets an assertion
failure or SIGSEGV.
I am inclined to fix this by decrementing md_num_open_segs before modifying
md_seg_fds (second attachment). An alternative would be to call
_fdvec_resize() after every FileClose(), like mdtruncate() does; however, the
repalloc() overhead could be noticeable. (mdclose() is called much more
frequently than mdtruncate().)
Incidentally, _mdfd_openseg() has this:
if (segno <= reln->md_num_open_segs[forknum])
_fdvec_resize(reln, forknum, segno + 1);
That should be >=, not <=. If the less-than case happened, this would delete
the record of a vfd for a higher-numbered segno. There's no live bug, because
only segno == reln->md_num_open_segs[forknum] actually happens. I am inclined
to make an assertion of that and remove the condition:
Assert(segno == reln->md_num_open_segs[forknum]);
_fdvec_resize(reln, forknum, segno + 1);
Attachment | Content-Type | Size |
---|---|---|
FileClose-test-v0.patch | text/plain | 3.7 KB |
mdclose-FileClose-v1.patch | text/plain | 551 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2019-12-22 11:34:19 | Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions |
Previous Message | Julien Rouhaud | 2019-12-22 08:06:41 | Re: [PATCH] Increase the maximum value track_activity_query_size |