On Fri, Sep 27, 2019 at 12:06 AM Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On Thu, Sep 26, 2019 at 06:58:17PM +0530, Amit Kapila wrote: > > >3. > >+ * Find the largest transaction (toplevel or subxact) to evict (spill to > >disk). > >+ * > >+ * 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%). > >+ */ > >+static ReorderBufferTXN * > >+ReorderBufferLargestTXN(ReorderBuffer *rb) > > > >What is the guarantee that after evicting largest transaction, we > >won't immediately hit the memory limit? Say, all of the transactions > >are of almost similar size which I don't think is that uncommon a > >case. > > Not sure I understand - what do you mean 'immediately hit'? > > We do check the limit after queueing a change, and we know that this > change is what got us over the limit. We pick the largest transaction > (which has to be larger than the change we just entered) and evict it, > getting below the memory limit again. > > The next change can get us over the memory limit again, of course, >
Yeah, this is what I want to say when I wrote that it can immediately hit again. > but > there's not much we could do about that. > > > Instead, the strategy mentioned in point (c) or something like > >that seems more promising. In that strategy, there is some risk that > >it might lead to many smaller disk writes which we might want to > >control via some threshold (like we should not flush more than N > >xacts). In this, we also need to ensure that the total memory freed > >must be greater than the current change. > > > >I think we have some discussion around this point but didn't reach any > >conclusion which means some more brainstorming is required. > > > > I agree it's worth investigating, but I'm not sure it's necessary before > committing v1 of the feature. I don't think there's a clear winner > strategy, and the current approach works fairly well I think. > > The comment is concerned with the cost of ReorderBufferLargestTXN with > many transactions, but we can only have certain number of top-level > transactions (max_connections + certain number of not-yet-assigned > subtransactions). And 0002 patch essentially gets rid of the subxacts > entirely, further reducing the maximum number of xacts to walk. > That would be good, but I don't understand how. The second patch will update the subxacts in top-level ReorderBufferTxn, but it won't remove it from hash table. It also doesn't seem to be caring for considering the size of subxacts in top-level xact, so not sure how will it reduce the number of xacts to walk. I might be missing something here. Can you explain a bit how 0002 patch would help in reducing the maximum number of xacts to walk? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
