Horton

Thread

Commits

  1. Fix memory leak and inefficiency in CREATE DATABASE ... STRATEGY WAL_LOG

  1. CREATE DATABASE ... STRATEGY WAL_LOG issues

    Andres Freund <andres@anarazel.de> — 2023-03-21T07:01:13Z

    Hi,

While hacking on my relation extension patch I found two issues with WAL_LOG:

1) RelationCopyStorageUsingBuffer() doesn't free the used strategies. This
   means we'll use #relations * ~10k memory

2) RelationCopyStorageUsingBuffer() gets the buffer for the target relation
   with RBM_NORMAL, therefore requiring a read of a block guaranteed to be
   zero

Easy enough to fix and shows clear improvement. One thing I wonder is if it's
worth moving the strategies up one level? Probaly not, but ...

Greetings,

Andres Freund

  2. Re: CREATE DATABASE ... STRATEGY WAL_LOG issues

    Robert Haas <robertmhaas@gmail.com> — 2023-03-21T15:33:59Z

    On Tue, Mar 21, 2023 at 3:01 AM Andres Freund <andres@anarazel.de> wrote:
> While hacking on my relation extension patch I found two issues with WAL_LOG:
>
> 1) RelationCopyStorageUsingBuffer() doesn't free the used strategies. This
>    means we'll use #relations * ~10k memory

Woops.

> 2) RelationCopyStorageUsingBuffer() gets the buffer for the target relation
>    with RBM_NORMAL, therefore requiring a read of a block guaranteed to be
>    zero

Woops.

> Easy enough to fix and shows clear improvement. One thing I wonder is if it's
> worth moving the strategies up one level? Probaly not, but ...

Hmm, so share a strategy across all relation forks? You could even
push it up a level beyond that and share it across all relations being
copied. That feels like it would be slightly more rational behavior,
but I'm not smart enough to guess whether anyone would actually be
happier (or less happy) after such a change than they are now.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  3. Re: CREATE DATABASE ... STRATEGY WAL_LOG issues

    Andres Freund <andres@anarazel.de> — 2023-03-21T16:34:14Z

    Hi,

On 2023-03-21 11:33:59 -0400, Robert Haas wrote:
> On Tue, Mar 21, 2023 at 3:01 AM Andres Freund <andres@anarazel.de> wrote:
> > Easy enough to fix and shows clear improvement. One thing I wonder is if it's
> > worth moving the strategies up one level? Probaly not, but ...
> 
> Hmm, so share a strategy across all relation forks? You could even
> push it up a level beyond that and share it across all relations being
> copied.

The latter is what I was wondering about.


> That feels like it would be slightly more rational behavior,
> but I'm not smart enough to guess whether anyone would actually be
> happier (or less happy) after such a change than they are now.

Yea, I'm not either. The current behaviour does have the feature that it will
read in some data for each table, but limits trashing of shared buffers for
huge tables. That's good if your small to medium sized source database isn't
in s_b, because the next CREATE DATABASE has a change to not need to read the
data again. But if you have a source database with lots of small relations, it
can easily lead to swamping s_b.

More generally, I still think we need logic to use unused buffers even when
strategies are in use (my current theory is that we wouldn't increase the
usagecount when strategies use unused buffers, so they can be replaced more
easily).

Greetings,

Andres Freund

  4. Re: CREATE DATABASE ... STRATEGY WAL_LOG issues

    Robert Haas <robertmhaas@gmail.com> — 2023-03-21T17:12:39Z

    On Tue, Mar 21, 2023 at 12:34 PM Andres Freund <andres@anarazel.de> wrote:
> More generally, I still think we need logic to use unused buffers even when
> strategies are in use

Yep.

> (my current theory is that we wouldn't increase the
> usagecount when strategies use unused buffers, so they can be replaced more
> easily).

Don't know about this part.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  5. Re: CREATE DATABASE ... STRATEGY WAL_LOG issues

    Andres Freund <andres@anarazel.de> — 2023-03-22T05:11:58Z

    Hi,

On 2023-03-21 09:34:14 -0700, Andres Freund wrote:
> On 2023-03-21 11:33:59 -0400, Robert Haas wrote:
> > That feels like it would be slightly more rational behavior,
> > but I'm not smart enough to guess whether anyone would actually be
> > happier (or less happy) after such a change than they are now.
> 
> Yea, I'm not either. The current behaviour does have the feature that it will
> read in some data for each table, but limits trashing of shared buffers for
> huge tables. That's good if your small to medium sized source database isn't
> in s_b, because the next CREATE DATABASE has a change to not need to read the
> data again. But if you have a source database with lots of small relations, it
> can easily lead to swamping s_b.

Patch with the two minimal fixes attached. As we don't know whether it's worth
changing the strategy, the more minimal fixes seem more appropriate.

Greetings,

Andres Freund

  6. Re: CREATE DATABASE ... STRATEGY WAL_LOG issues

    Robert Haas <robertmhaas@gmail.com> — 2023-03-22T13:58:58Z

    On Wed, Mar 22, 2023 at 1:12 AM Andres Freund <andres@anarazel.de> wrote:
> Patch with the two minimal fixes attached. As we don't know whether it's worth
> changing the strategy, the more minimal fixes seem more appropriate.

LGTM.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  7. Re: CREATE DATABASE ... STRATEGY WAL_LOG issues

    Andres Freund <andres@anarazel.de> — 2023-03-22T17:04:19Z

    On 2023-03-22 09:58:58 -0400, Robert Haas wrote:
> On Wed, Mar 22, 2023 at 1:12 AM Andres Freund <andres@anarazel.de> wrote:
> > Patch with the two minimal fixes attached. As we don't know whether it's worth
> > changing the strategy, the more minimal fixes seem more appropriate.
> 
> LGTM.

Thanks for checking. Pushed.