On Wed, Mar 20, 2019 at 05:17:54PM +0900, Kyotaro HORIGUCHI wrote:
> At Sun, 10 Mar 2019 19:27:08 -0700, Noah Misch <noah@leadboat.com> wrote in
<20190311022708.GA2189728@rfd.leadboat.com>
> > On Mon, Mar 04, 2019 at 12:24:48PM +0900, Kyotaro HORIGUCHI wrote:
> > > +/*
> > > + * Sync to disk any relations that we skipped WAL-logging for earlier.
> > > + */
> > > +void
> > > +smgrDoPendingSyncs(bool isCommit)
> > > +{
> > > + if (!pendingSyncs)
> > > + return;
> > > +
> > > + if (isCommit)
> > > + {
> > > + HASH_SEQ_STATUS status;
> > > + PendingRelSync *pending;
> > > +
> > > + hash_seq_init(&status, pendingSyncs);
> > > +
> > > + while ((pending = hash_seq_search(&status)) != NULL)
> > > + {
> > > + if (pending->sync_above != InvalidBlockNumber)
> >
> > I'm mildly unhappy that pendingSyncs entries with "pending->sync_above ==
> > InvalidBlockNumber" are not sync requests at all. Those just record the fact
> > of a RelationTruncate() happening. If you can think of a way to improve that,
> > please do so. If not, it's okay.
>
> After a truncation, required WAL records are emitted for the
> truncated pages, so no need to sync. Does this make sense for
> you? (Maybe commit is needed there)
Yes, the behavior makes sense. I wasn't saying the quoted code had the wrong
behavior. I was saying that the data structure called "pendingSyncs" is
actually "pending syncs and past truncates". It's not ideal that the variable
name differs from the variable purpose in this way. However, it's okay if you
don't find a way to improve that.
> I don't have enough time for now so the new version will be
> posted early next week.
I'll wait for that version.