On Fri, Jul 19, 2019 at 6:35 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Jul 19, 2019 at 12:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > We are doing exactly what you have written in the last line of the
> > next paragraph "stop the transaction from writing undo when the hash
> > table is already too full.". So we will
> > never face the problems related to repeated crash recovery. The
> > definition of too full is that we stop allowing the new transactions
> > that can write undo when we have the hash table already have entries
> > equivalent to (UndoRollbackHashTableSize() - MaxBackends). Does this
> > make sense?
>
> Oops, I was looking in the wrong place. Yes, that makes sense, but:
>
> 1. It looks like the test you added to PrepareUndoInsert has no
> locking, and I don't see how that can be right.
>
+if (ProcGlobal->xactsHavingPendingUndo >
+(UndoRollbackHashTableSize() - MaxBackends))
Actual HARD_LIMIT is UndoRollbackHashTableSize(), but we only allow a
new backend to prepare the undo record if we have MaxBackends number
of empty slots in the hash table. This will guarantee us to always
have at least one slot in the hash table for our current prepare, even
if all the backend which running their transaction has aborted and
inserted an entry in the hash table.
I think the problem with this check is that for any backend to prepare
an undo there must be MaxBackend number of empty slots in the hash
table for any concurrent backend to insert the request and this seems
too restrictive.
Having said that I think we must ensure MaxBackends*2 empty slots in
the hash table as each backend can enter 2 requests in the hash table.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com