Re: Error with index on unlogged table - Mailing list pgsql-hackers

On Tue, Dec 8, 2015 at 10:09 PM, Andres Freund <andres@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 <andres@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