RE: Copy data to DSA area

From: "Ideriha, Takeshi" <ideriha(dot)takeshi(at)jp(dot)fujitsu(dot)com>
To: 'Kyotaro HORIGUCHI' <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "thomas(dot)munro(at)enterprisedb(dot)com" <thomas(dot)munro(at)enterprisedb(dot)com>, "robertmhaas(at)gmail(dot)com" <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: Copy data to DSA area
Date: 2019-04-26 14:50:00
Message-ID: 4E72940DA2BF16479384A86D54D0988A7DB313E1@G01JPEXMBKW04
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, I've updated Thomas's quick PoC.

>From: Ideriha, Takeshi [mailto:ideriha(dot)takeshi(at)jp(dot)fujitsu(dot)com]
>Sent: Wednesday, April 17, 2019 2:07 PM
>>From: Ideriha, Takeshi [mailto:ideriha(dot)takeshi(at)jp(dot)fujitsu(dot)com]
>>Sent: Wednesday, December 5, 2018 2:42 PM
>>Subject: RE: Copy data to DSA area
>
>Things under discussion:
>- How we prevent memory leak
>- How we prevent dangling pointer after cleaning up about-to-leak-objects
>
>Regarding memory leak, I think Robert's idea that allocate objects under temporal
>context while building and re-parent it to permanent one at some point is promising.
>While building objects they are under temporal DSA-MemoryContext, which is child of
>TopTransactionContext (if it's in the transaction) and are freed all at once when error
>happens.
>To do delete all the chunks allocated under temporal DSA context, we need to search
>or remember all chunks location under the context. Unlike AllocAset we don't have
>block information to delete them altogether.
>
>So I'm thinking to manage dsa_allocated chunks with single linked list to keep track of
>them and delete them.
>The context has head of linked list and all chunks have pointer to next allocated chunk.
>But this way adds space overhead to every dsa_allocated chunk and we maybe want
>to avoid it because shared memory size is limited.
>In this case, we can free these pointer area at some point when we make sure that
>allocation is successful.
>
>Another debate is when we should think the allocation is successful (when we make
>sure object won't leak).
>If allocation is done in the transaction, we think if transaction is committed we can
>think it's safe.
>Or I assume this DSA memory context for cache such as relcache, catcache, plancache
>and so on.
>In this case cache won't leak once it's added to hash table or list because I assume
>some eviction mechanism like LRU will be implemented and it will erase useless cache
>some time later.

I changed design from my last email.
I've introduced "local" and "shared" MemoryContext for DSA.
Local means MemoryContext object is allocated on local heap and shared means on shared memory.
While dsa_allocating, operation should be done on local context and after developer thinks
they don't leak, set local context to shared one.

Dsa_pointer to chunks is stored in the array linked by local context.
When error happens before chunks become "shared", all chunks are freed by checking the array.
On the other hand, chunk gets "committed" and become shared, we don't need that array of pointers.

This PoC has three test cases, which is updated version of Thomas's ones.
- hoge() : palloc(), pfree() then set the context shared and do it again
- hoge_list(): add chunk to shared single linked list and set context to shared
- hoge_list_error(): add chunk to linked list but error happens before it becomes shared one,
so free the chunk to prevent memory leak

Well, after developing PoC, I realized that this PoC doesn't solve the local process is crashed before
the context becomes shared because local process keeps track of pointer to chunks.
Maybe all of you have already noticed and pointed out this case :)
So it needs another work but this poc is a good step for me to advance more.

Another thing is that I don't want put backpointer to MemoryContext before every chunks
since it has some overhead in limited shared memory. But pfree uses it so I compromised it.
And when set local context to shared one, I need to change every backpointer to shared MemoryContext
so it has some cost.

I think there is more efficient way.
(Maybe Robert mentioned it in previous email?)

>Regarding dangling pointer I think it's also problem.
>After cleaning up objects to prevent memory leak we don't have mechanism to reset
>dangling pointer.

I haven't addressed the dangling pointer yet.
Actually hoge_list() issued after hoge_list_error() is executed leads backends crash.
That's because it seems to me that allocated chunk is freed after error
but the tail pointer of shared linked list is not recovered.
It becomes dangling pointer.
So this would be a good example of dangling pointer.

Regards,
Takeshi Ideriha

Attachment Content-Type Size
PoC-DSA-MemoryContext-extension.tar.gz application/x-gzip 3.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Rafia Sabih 2019-04-26 14:51:26 Re: Execute INSERT during a parallel operation
Previous Message Michael Paquier 2019-04-26 14:38:39 Re: Fwd: Add tablespace tap test to pg_rewind