Re: WIP: Pg_upgrade - page layout converter (PLC) hook

Zdenek Kotala wrote:
> Heikki Linnakangas napsal(a):
>> I would suggest starting with putting some serious effort into
>> pg_migrator. This page layout conversion is actually pretty trivial
>> compared to all that, and even more so if you can get the page layout
>> conversion working in pg_migrator first.
>
> By my opinion pg_migrator is good workaround but it is not suitable for
> real production. For example TOAST relid protection is dirty hack and
> you have problem with tablespaces and so on...

Sure, it's not perfect, that's exactly why I suggested working on it! If
you have something else that works better, that's fine, but you will
need to release it under the BSD license if you want help with it.

>> Which versions do you plan to support, initially?
>
> Currently I'm able to upgrade from 8.1, 8.2 to 8.3, 8.4. It means
> conversion between layout version 3 and 4. I'm verifying PLC now and
> when this part will be ready it is very easy to implement others as well.

Hmm. I don't see any of that code in the directory you posted. ?

>>> b) log_newpage is used for new page logging, but I use it for
>>> storing converted page. It seems to me that it safe and
>>> heap_xlog_newpage correctly works for new and converted page. I have
>>> only doubt about assert macro mdextend/mdwrite which checks extend
>>> vs.write.
>>
>> That should be fine. In WAL replay, we don't distinguish between write
>> and extend.
>
> But in this case the asserts macros in mdexted/mdwrite are odd and the
> should be removed.

Those asserts are enforced during normal operation. It's just in WAL
replay that we extend the relation automatically when full pages are
restored. See XLogReadBuffer().

>>> + /* Hook for page layout convertor plugin */
>>> + typedef void (*plc_hook_type)(char *buffer);
>>> + extern PGDLLIMPORT plc_hook_type plc_hook;
>>
>> That's not flexible enough. We've changed the internal representations
>> of data types in the past, and will likely do that in the future. The
>> hook therefore needs to have at least the tuple descriptor, to know
>> how to convert the tuples. I would suggest passing Relation, we have
>> that available at the call site, and that should contain all the
>> necessary information.
>
> Good point, I thought about it. I currently don't use it but in the
> future it could be useful. I will extend it.

Surely you need it already to do the packed varlen conversion?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com