Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)
| От | Pavan Deolasee |
|---|---|
| Тема | Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM) |
| Дата | |
| Msg-id | CABOikdOvTLp5qYtoN+svfdHAcSR2x=yt_JcoohQhdv6pmnkWrA@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM) (Jaime Casanova <jaime.casanova@2ndquadrant.com>) |
| Ответы |
Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)
|
| Список | pgsql-hackers |
On Mon, Dec 26, 2016 at 11:49 AM, Jaime Casanova <jaime.casanova@2ndquadrant.com> wrote:
-- On 2 December 2016 at 07:36, Pavan Deolasee <pavan.deolasee@gmail.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.sql@15 -f updates_1.sql@20 -f
updates_2.sql@20 -f deletes.sql@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
PostgreSQL Development, 24x7 Support, Training & Services
Вложения
В списке pgsql-hackers по дате отправления: