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)  (Pavan Deolasee <pavan.deolasee@gmail.com>)
Список 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
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Aleksander Alekseev
Дата:
Сообщение: Re: [HACKERS] PATCH: recursive json_populate_record()
Следующее
От: Ashutosh Bapat
Дата:
Сообщение: Re: [HACKERS] ALTER TABLE parent SET WITHOUT OIDS and the oid column