Re: Error with index on unlogged table

On Tue, Dec 8, 2015 at 10:09 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2015-12-04 21:57:54 +0900, Michael Paquier wrote:
>> On Fri, Dec 4, 2015 at 5:10 PM, Andres Freund <andres(at)anarazel(dot)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