Re: Error with index on unlogged table

On 2015-12-04 17:00:13 +0900, Michael Paquier wrote:
> Andres Freud wrote:
> >> @@ -450,14 +450,21 @@ ginbuildempty(PG_FUNCTION_ARGS)
> >> START_CRIT_SECTION();
> >> GinInitMetabuffer(MetaBuffer);
> >> MarkBufferDirty(MetaBuffer);
> >> - log_newpage_buffer(MetaBuffer, false);
> >> + log_newpage_buffer(MetaBuffer, false, false);
> >> GinInitBuffer(RootBuffer, GIN_LEAF);
> >> MarkBufferDirty(RootBuffer);
> >> - log_newpage_buffer(RootBuffer, false);
> >> + log_newpage_buffer(RootBuffer, false, true);
> >> END_CRIT_SECTION();
> >>
> > Why isn't the metapage flushed to disk?
>
> I was not sure if we should only flush only at the last page, as pages
> just before would be already replayed. Switched.

Uh, that's not how it works! The earlier pages would just be in shared
buffers. Neither smgrwrite, nor smgrimmedsync does anything about that!

> >> extern void InitXLogInsert(void);
> >> diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
> >> index ad1eb4b..91445f1 100644
> >> --- a/src/include/catalog/pg_control.h
> >> +++ b/src/include/catalog/pg_control.h
> >> @@ -73,6 +73,7 @@ typedef struct CheckPoint
> >> #define XLOG_END_OF_RECOVERY 0x90
> >> #define XLOG_FPI_FOR_HINT 0xA0
> >> #define XLOG_FPI 0xB0
> >> +#define XLOG_FPI_FOR_SYNC 0xC0
> >
> >
> > I'm not a big fan of the XLOG_FPI_FOR_SYNC name. Syncing is a bit too
> > ambigous for my taste. How about either naming it XLOG_FPI_FLUSH or
> > instead adding actual record data and a 'flags' field in there? I
> > slightly prefer the latter - XLOG_FPI and XLOG_FPI_FOR_HINT really are
> > different, XLOG_FPI_FOR_SYNC not so much.
>
> 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.

> diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
> index 99337b0..b646101 100644
> --- a/src/backend/access/brin/brin.c
> +++ b/src/backend/access/brin/brin.c
> @@ -682,7 +682,15 @@ brinbuildempty(PG_FUNCTION_ARGS)
> brin_metapage_init(BufferGetPage(metabuf), BrinGetPagesPerRange(index),
> BRIN_CURRENT_VERSION);
> MarkBufferDirty(metabuf);
> - log_newpage_buffer(metabuf, false);
> +
> + /*
> + * For unlogged relations, this page should be immediately flushed
> + * to disk after being replayed. This is necessary to ensure that the
> + * initial on-disk state of unlogged relations is preserved when
> + * they get reset at the end of recovery.
> + */
> + log_newpage_buffer(metabuf, false,
> + index->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED);
> END_CRIT_SECTION();

Maybe write the last sentence as '... as the on disk files are copied
directly at the end of recovery.'?

> @@ -336,7 +336,8 @@ end_heap_rewrite(RewriteState state)
> MAIN_FORKNUM,
> state->rs_blockno,
> state->rs_buffer,
> - true);
> + true,
> + false);
> RelationOpenSmgr(state->rs_new_rel);
>
> PageSetChecksumInplace(state->rs_buffer, state->rs_blockno);
> @@ -685,7 +686,8 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
> MAIN_FORKNUM,
> state->rs_blockno,
> page,
> - true);
> + true,
> + false);

Did you verify that that's ok when a unlogged table is clustered/vacuum
full'ed?

> @@ -181,6 +183,9 @@ xlog_identify(uint8 info)
> case XLOG_FPI_FOR_HINT:
> id = "FPI_FOR_HINT";
> break;
> + case XLOG_FPI_FLUSH:
> + id = "FPI_FOR_SYNC";
> + break;
> }

Old string.

> --- a/src/backend/access/transam/xloginsert.c
> +++ b/src/backend/access/transam/xloginsert.c
> @@ -932,10 +932,13 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std)
> * If the page follows the standard page layout, with a PageHeader and unused
> * space between pd_lower and pd_upper, set 'page_std' to TRUE. That allows
> * the unused space to be left out from the WAL record, making it smaller.
> + *
> + * If 'is_flush' is set to TRUE, relation will be requested to flush
> + * immediately its data at replay after replaying this full page image.
> */

s/is_flush/flush_immed/? And maybe say that it 'will be flushed to the
OS immediately after replaying the record'?

Greetings,

Andres Freund