Philip Warner <pjw at rhyme.com.au> writes:
> Thanks for the reply.
>
> Christopher Browne wrote:
>
>> Hey, if we can change the behaviour of slon so that it consumes Very
>> Little Memory, or, rather, to prevent it from Behaving Badly, that
>> hardly seems a bad thing.
>
> That's what I'd like to do. If a slon process is killed, or a link goes
> down too long, I end up with a DB that can not be made 'current'.
> Sometimes, I just get a big update, and slon dies (and will not restart).
>
>> (e.g. - when sl_log_1 entries are
>>being applied, they are done in sets of 10) involves some records that
>>are Very Large.
>>  
>>
> Indeed; we can have a single record of 37MB (max).

Ah, good, so I'm not heading down a wrong road on this.

>>--> If size of current data being processed > 10MB then don't
>>    bother moving on from record 7 to record 8; push the 
>>    data out NOW and clear the data structure...

> This is the kind of approach I was thinking of; just stop fetching,
> and wake up the code that does the replication.

Alas, that begs a "how do we do that?"

In this area, Slony-I isn't doing any magic.  It just does a plain old
"FETCH [X] FROM LOG" statement, which does _not_ admit any opportunity
to stop fetching early.

>>The only downside that I see is the cost of searching through the
>>strings to see how big they are.

> Shouldn't be a problem; AFAICT the rows are fetched and stored in a
> queue which does a realloc to store the data. Must know the size.

That works for the "submission" side, where slon is generating the
queries that are to go into the destination system.

>>That's with the "FETCH 100 FROM LOG" queries.  That's going to draw in
>>100 rows from sl_log_1 each time, and if some are Pathological Big
>>Records, _there_ lies the problem.

> I think that's part of the problem; we could fetch 10 at a time, but
> the way read the code, it will just loop and fetch the next 10. So
> either way we'll consume lots of memory.
>
> My proposed solution is to:
>
> (a) use the 'max buffer space' as a guideline only
> (b) use fetch 1 or fetch 10 (they both seem fast)
> (c) when the current *used* buffer space (ie buffers in the
> 'to-be-processed' queue) exceeds the buffer limit (after the fetch-10 is
> complete), then don't fetch the next 10. Wait for the used buffer space
> to drop below an 'empty/restart-limit'.
> (d) dealloc/free memory from the queues once processed (if not already done)
>
> This means that the buffer limit would always be exceed, but by at most
> 10 fetches.If every one of those 10 log rows is 37MB, then I'll blow my
> process address size again, but that is *very* unlikely.
>
> Running a query to get the strig sizes (as you may have suggested
> above, now I think about it) could/would prevent problems: "select
> sum(length(log_cmddata)) from...". We could then adjust the fetch
> size to 'fetch 1' if the total size will blow the current buffer
> size.

That query would be exceedingly expensive for the scenario where there
are a million rows outstanding in sl_log_1.

Indeed, the logic that you're proposing is a logic which would worsen
the behaviour of Slony-I for use cases where records are generally
pretty small.

Consider the scenario where none of the sl_log_1 records are larger
than 1K in size, but where we have *enormous* numbers of them.

Using FETCH 1 rather than FETCH 100 means submitting 100 times as many
queries, and having 100 times as many query round trips between the
slon and the provider.

>>This is NOT easily changeable at runtime, but if you're running into
>>these problems, then I'd suggest decreasing these values in slon.h.

> Why is that? There are a lot of places that reference the values,
> but is there a real problem with making them variable?

They can't be variables because their values are used at compile time
to indicate array sizes.  It is certainly possible to change the array
definitions and use malloc() at runtime, but that involves quite a bit
of change.

>>Ideally, it would be nice for slon to figure out suitable values by
>>itself.  How to do that when a bad choice would lead to slon running
>>out of memory and falling over seems, erm, troublesome :-(.

> Using the "select sum(length(log_cmddata)) from..." approach, then
> using either fetch-100 or fetch-1 based on the result seems like a
> good first pass. Also has the advantage of no change from the users
> perspective unless they have huge queries -- and then the change
> improves reliability.

... Unless there *aren't* any Fat Rows, in which reliability goes down
because we're submitting flurries of tiny queries that take longer to
run making it more likely that replication falls behind.

> To summarize, the basic approach would be:
>
>  - prior to doing an open cursor, run "select sum(length(log_cmddata))
> from...limit 100".
>  - use this to determine fetch size (1 or 100).
>  - open cursor/fetch etc. At the end of the fetch loop, see how much
> buffered data we have in the queues. If too much, pause and store in
> replicated DB.
>  - repeat.
>
> Does this sound broadly OK? Not sure how best to do the pause/restart.
> Just sleep/wake? Use thread events? etc etc

No, there is no pause/restart.  The fetch size determines everything.

> Later, this could be refined by selecting lengths of next 100 and
> tuning how many we fetch based on this knowledge. But as a first
> pass it seems to satisfy the requirements.

I really dislike the way that this injures behaviour for
non-pathological cases (e.g.  - where there aren't Fat Rows 37MB in
size).

Not to be too pointed about it, but our systems don't have Fat Rows,
and I'm quite disinclined to make changes that specifically hurt us...

I have a different thought, which might conceivably improve things for
everyone...

The thought is to store record size in a new field on sl_log_1, call
it "log_cmdsize", and populate it at insert time.

We declare LOG cursor as being ...

declare LOG cursor for select log_origin, log_xid, log_tableid, 
       log_actionseq, log_cmdtype, log_cmdsize
from sl_log_1 [with various other criteria] order by log_actionseq;

On this cursor, we can probably quite well afford to blindly do FETCH
1000.

We then walk through the 1000 rows, decide how many we'll be applying,
based on a logic like...

for each row do
   add the row's key information to a query to pull *ALL* data from sl_log_1
   ram_consumption += row.log_cmdsize
   if ram_consumption > threshold then
      perform the second query and load results into this node
   end if
done

In cases where log_cmddata is small, this would mean that we might do
way *more* than 10 at a time, which should mitigate the performance
loss resulting from having to read thru sl_log_1 twice.

The mechanism for efficiently pulling the detail data from sl_log_1
based on a set of keys probably requires generating a 2D array of
index values to pass in to a set-returning stored procedure.

I really see two answers to this, at this point...

1.  Set the FETCH value(s) to 1 rather than 100 at compile time if you
know you have problems with Fat Rows.

This is, of course, a reasonable immediate answer that would be
available to you in 1.0.5.

2.  We have to do something quite a bit cleverer, probably similar to
what I outlined, if we don't want to injure users that don't use Fat
Rows.

Cleverer can't be in place until 1.2, at the earliest...

I need to talk with a couple people about this...
-- 
(format nil "~S@~S" "cbbrowne" "ca.afilias.info")
<http://dev6.int.libertyrms.com/>
Christopher Browne
(416) 673-4124 (land)