public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: In-placre persistance change of a relation
Date: Fri, 13 Nov 2020 16:47:48 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <TYAPR01MB2990D25E5CE77A095ADC90A3FEE60@TYAPR01MB2990.jpnprd01.prod.outlook.com>
References: <[email protected]>
<[email protected]>
<TYAPR01MB2990D25E5CE77A095ADC90A3FEE60@TYAPR01MB2990.jpnprd01.prod.outlook.com>
At Fri, 13 Nov 2020 06:43:13 +0000, "[email protected]" <[email protected]> wrote in
> Hi Horiguchi-san,
>
>
> Thank you for making a patch so quickly. I've started looking at it.
>
> What makes you think this is a PoC? Documentation and test cases? If there's something you think that doesn't work or are concerned about, can you share it?
The latest version is heavily revised and is given much comment so it
might have exited from PoC state. The necessity of documentation is
doubtful since this patch doesn't user-facing behavior other than
speed. Some tests are required especialy about recovery and
replication perspective but I haven't been able to make it. (One of
the tests needs to cause crash while a transaction is running.)
> Do you know the reason why data copy was done before? And, it may be odd for me to ask this, but I think I saw someone referred to the past discussion that eliminating data copy is difficult due to some processing at commit. I can't find it.
To imagine that, just because it is simpler considering rollback and
code sharing, and maybe no one have been complained that SET
LOGGED/UNLOGGED looks taking a long time than required/expected.
The current implement is simple. It's enough to just discard old or
new relfilenode according to the current transaction ends with commit
or abort. Tweaking of relfilenode under use leads-in some skews in
some places. I used pendingDelete mechanism a bit complexified way
and a violated an abstraction (I think, calling AM-routines from
storage.c is not good.) and even introduce a new fork kind only to
mark a init fork as "not committed yet". There might be better way,
but I haven't find it.
(The patch scans all shared buffer blocks for each relation).
> (1)
> @@ -168,6 +168,8 @@ extern PGDLLIMPORT int32 *LocalRefCount;
> */
> #define BufferGetPage(buffer) ((Page)BufferGetBlock(buffer))
>
> +struct SmgrRelationData;
>
> This declaration is already in the file:
>
> /* forward declared, to avoid having to expose buf_internals.h here */
> struct WritebackContext;
>
> /* forward declared, to avoid including smgr.h here */
> struct SMgrRelationData;
Hmmm. Nice chatch. And will fix in the next version.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
view thread (54+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: In-placre persistance change of a relation
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox