|From:||Heikki Linnakangas <hlinnaka(at)iki(dot)fi>|
|To:||Thomas Munro <thomas(dot)munro(at)gmail(dot)com>|
|Subject:||Re: pendingOps table is not cleared with fsync=off|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 09/05/2020 02:53, Thomas Munro wrote:
> On Sat, May 9, 2020 at 9:21 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> I noticed that commit 3eb77eba5a changed the logic in
>> ProcessSyncRequests() (formerly mdsync()) so that if you have fsync=off,
>> the entries are not removed from the pendingOps hash table. I don't
>> think that was intended.
> Perhaps we got confused about what the comment "... so that changing
> fsync on the fly behaves sensibly" means. Fsyncing everything you
> missed when you turn it back on after a period running with it off
> does sound a bit like behaviour that someone might want or expect,
> though it probably isn't really enough to guarantee durability,
> because requests queued here aren't the only fsyncs you missed while
> you had it off, among other problems.
Yeah, you need to run "sync" after turning fsync=on to be safe, there's
no way around it.
> Unfortunately, if you try that on an assertion build, you get a
> failure anyway, so that probably wasn't deliberate:
> TRAP: FailedAssertion("(CycleCtr) (entry->cycle_ctr + 1) ==
> sync_cycle_ctr", File: "sync.c", Line: 335)
Ah, I didn't notice that.
>> I propose the attached patch to move the test for enableFsync so that
>> the entries are removed from pendingOps again. It looks larger than it
>> really is because it re-indents the block of code that is now inside the
>> "if (enableFsync)" condition.
> Yeah, I found that git diff/show -w made it easier to understand that
> change. LGTM, though I'd be tempted to use "goto skip" instead of
> incurring that much indentation but up to you ...
I considered a goto too, but I found it confusing. If we need any more
nesting here in the future, I think extracting the inner parts into a
function would be good.
|Next Message||Amit Kapila||2020-05-14 05:41:46||Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions|
|Previous Message||Fabien COELHO||2020-05-14 05:23:05||Re: PG 13 release notes, first draft|