Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

> On 25 December 2017 at 18:40, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
wrote:
> The attached v3 fixes this issue, and also a couple of other thinkos

Thank you for the patch, it looks quite interesting. After a quick look at
it
(mostly the first one so far, but I'm going to continue) I have a few
questions:

> + * XXX With many subtransactions this might be quite slow, because we'll
have
> + * to walk through all of them. There are some options how we could
improve
> + * that: (a) maintain some secondary structure with transactions sorted
by
> + * amount of changes, (b) not looking for the entirely largest
transaction,
> + * but e.g. for transaction using at least some fraction of the memory
limit,
> + * and (c) evicting multiple transactions at once, e.g. to free a given
portion
> + * of the memory limit (e.g. 50%).

Do you want to address these possible alternatives somehow in this patch or
you
want to left it outside? Maybe it makes sense to apply some combination of
them, e.g. maintain a secondary structure with relatively large
transactions,
and then start evicting them. If it's somehow not enough, then start to
evict
multiple transactions at once (option "c").

> + /*
> + * We clamp manually-set values to at least 64kB. The
maintenance_work_mem
> + * uses a higher minimum value (1MB), so this is OK.
> + */
> + if (*newval < 64)
> + *newval = 64;
> +

I'm not sure what's recommended practice here, but maybe it makes sense to
have a warning here about changing this value to 64kB? Otherwise it can be
unexpected.