On Tue, Dec 8, 2015 at 10:09 PM, Andres Freund <and...@anarazel.de> wrote: > On 2015-12-04 21:57:54 +0900, Michael Paquier wrote: >> On Fri, Dec 4, 2015 at 5:10 PM, Andres Freund <and...@anarazel.de> wrote: >> >> Let's go for XLOG_FPI_FLUSH. >> > >> > I think the other way is a bit better, because we can add new flags >> > without changing the WAL format. >> >> Hm. On the contrary, I think that it would make more sense to have a >> flag as well for FOR_HINT honestly, those are really the same >> operations, and FOR_HINT is just here for statistic purposes. > > I don't think it's all that much the same operation. And WRT statistics > purpose: Being able to easily differentiate FOR_HINT is important for > pg_xlogdump --stats, but not at all for XLOG_FPI_FLUSH.
OK. Switched back to using XLOG_FPI_FLUSH.
>> [stuff about unlogged relations in code]
>
> I might be missing something here - but isn't it pretty much guaranteed
> that all these are unlogged relations? Otherwise *buildempty() wouldn't
> have been called, right?
Yep, that's ensured in index.c when calling ambuildempty. Let's flush
it unconditionally then in those code paths.
>> else if (info == XLOG_BACKUP_END)
>> {
>> @@ -178,9 +183,6 @@ xlog_identify(uint8 info)
>> case XLOG_FPI:
>> id = "FPI";
>> break;
>> - case XLOG_FPI_FOR_HINT:
>> - id = "FPI_FOR_HINT";
>> - break;
>> }
>
> Really don't want to do that.
OK, added back.
>> @@ -9391,14 +9394,34 @@ xlog_redo(XLogReaderState *record)
>> * resource manager needs to generate conflicts, it has to
>> define a
>> * separate WAL record type and redo routine.
>> *
>> - * XLOG_FPI_FOR_HINT records are generated when a page needs
>> to be
>> - * WAL- logged because of a hint bit update. They are only
>> generated
>> - * when checksums are enabled. There is no difference in
>> handling
>> - * XLOG_FPI and XLOG_FPI_FOR_HINT records, they use a
>> different info
>> - * code just to distinguish them for statistics purposes.
>> + * Records flagged with 'for_hint_bits' are generated when a
>> page needs
>> + * to be WAL- logged because of a hint bit update. They are
>> only
>> + * generated when checksums are enabled. There is no
>> difference in
>> + * handling records when this flag is set, it is used for
>> statistics
>> + * purposes.
>> + *
>> + * Records flagged with 'is_flush' indicate that the page
>> immediately
>> + * needs to be written to disk, not just to shared buffers.
>> This is
>> + * important if the on-disk state is to be the authoritative,
>> not the
>> + * state in shared buffers. E.g. because on-disk files may
>> later be
>> + * copied directly.
>> */
>> if (XLogReadBufferForRedo(record, 0, &buffer) != BLK_RESTORED)
>> elog(ERROR, "unexpected XLogReadBufferForRedo result
>> when restoring backup block");
>> +
>> + if (xlrec.is_flush)
>> + {
>> + RelFileNode rnode;
>> + ForkNumber forknum;
>> + BlockNumber blkno;
>> + SMgrRelation srel;
>> +
>> + (void) XLogRecGetBlockTag(record, 0, &rnode, &forknum,
>> &blkno);
>> + srel = smgropen(rnode, InvalidBackendId);
>> + smgrwrite(srel, forknum, blkno, BufferGetPage(buffer),
>> false);
>> + smgrclose(srel);
>> + }
>
> That'd leave the dirty bit set on the buffer...
OK, I have used an equivalent of FlushBuffer, FlushSingleBuffer being
a copycat of your previous patch... I am attaching as well a patch for
~9.4, which uses XLOG_HEAP_NEWPAGE instead.
--
Michael
20151209_replay_unlogged_master_95.patch
Description: binary/octet-stream
20151209_replay_unlogged_94.patch
Description: binary/octet-stream
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
