Skip site navigation (1) Skip section navigation (2)

Re: Patch: Write Amplification Reduction Method (WARM)

From: Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
To: Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>
Cc: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch: Write Amplification Reduction Method (WARM)
Date: 2016-12-27 13:21:48
Message-ID: CABOikdOvTLp5qYtoN+svfdHAcSR2x=yt_JcoohQhdv6pmnkWrA@mail.gmail.com (view raw, whole thread or download thread mbox)
Thread:
Lists: pgsql-hackers
On Mon, Dec 26, 2016 at 11:49 AM, Jaime Casanova <
jaime(dot)casanova(at)2ndquadrant(dot)com> wrote:

> On 2 December 2016 at 07:36, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
> wrote:
> >
> > I've updated the patches after fixing the issue. Multiple rounds of
> > regression passes for me without any issue. Please let me know if it
> works
> > for you.
> >
>
> Hi Pavan,
>
> Today i was playing with your patch and running some tests and found
> some problems i wanted to report before i forget them ;)
>
>
Thanks Jaime for the tests and bug reports. I'm attaching an add-on patch
which fixes these issues for me. I'm deliberately not sending a fresh
revision because the changes are still minor.


> * You need to add a prototype in src/backend/utils/adt/pgstatfuncs.c:
> extern Datum pg_stat_get_tuples_warm_updated(PG_FUNCTION_ARGS);
>

Added.


>
> * The isolation test for partial_index fails (attached the
> regression.diffs)
>

Fixed. Looks like I forgot to include attributes from predicates and
expressions in the list of index attributes (as pointed by Alvaro)


>
> * running a home-made test i have at hand i got this assertion:
> """
> TRAP: FailedAssertion("!(buf_state & (1U << 24))", File: "bufmgr.c", Line:
> 837)
> LOG:  server process (PID 18986) was terminated by signal 6: Aborted
> """
> To reproduce:
> 1) run prepare_test.sql
> 2) then run the following pgbench command (sql scripts attached):
> pgbench -c 24 -j 24 -T 600 -n -f inserts(dot)sql(at)15 -f updates_1(dot)sql(at)20 -f
> updates_2(dot)sql(at)20 -f deletes(dot)sql(at)45 db_test
>
>
Looks like the patch was failing to set the block number correctly in the
t_ctid field, leading to these strange failures. There was also couple of
instances where the t_ctid field was being accessed directly, instead of
the newly added macro. I think we need some better mechanism to ensure that
we don't miss out on such things. But I don't have a very good idea about
doing that right now.


>
> * sometimes when i have made the server crash the attempt to recovery
> fails with this assertion:
> """
> LOG:  database system was not properly shut down; automatic recovery in
> progress
> LOG:  redo starts at 0/157F970
> TRAP: FailedAssertion("!(!warm_update)", File: "heapam.c", Line: 8924)
> LOG:  startup process (PID 14031) was terminated by signal 6: Aborted
> LOG:  aborting startup due to startup process failure
> """
> still cannot reproduce this one consistently but happens often enough
>
>
This could be a case of uninitialised variable in log_heap_update(). What
surprises me though that none of the compilers I tried so far could catch
that. In the following code snippet, if the condition evaluates to false
then "warm_update" may remain uninitialised, leading to wrong xlog entry,
which may later result in assertion failure during redo recovery.

7845
7846     if (HeapTupleIsHeapWarmTuple(newtup))
7847         warm_update = true;
7848

Thanks,
Pavan

-- 
 Pavan Deolasee                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Attachment: 0003_warm_fixes_v6.patch
Description: application/octet-stream (3.7 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Ashutosh BapatDate: 2016-12-27 13:24:56
Subject: Re: ALTER TABLE parent SET WITHOUT OIDS and the oid column
Previous:From: Aleksander AlekseevDate: 2016-12-27 13:15:41
Subject: Re: PATCH: recursive json_populate_record()

Privacy Policy | About PostgreSQL
Copyright © 1996-2018 The PostgreSQL Global Development Group