On Wed, Aug 3, 2022, at 10:27 AM, Amit Kapila wrote:
Your explanation makes sense to me. The other point to consider is
that there can be cases where we may not apply operation for the
transaction because of empty transactions (we don't yet skip empty
xacts for prepared transactions). So, won't it be better to apply the
delay just before we apply the first change for a transaction? Do we
want to apply the delay during table sync as we sometimes do need to
enter apply phase while doing table sync?
I thought about the empty transactions but decided to not complicate the code
mainly because skipping transactions is not a code path that will slow down
this feature. As explained in the documentation, there is no harm in delaying a
transaction for more than min_apply_delay; it cannot apply earlier. Having said
that I decided to do nothing. I'm also not sure if it deserves a comment or if
this email is a possible explanation for this decision.
Regarding the table sync that was mention by Melih, I sent a new version (v6)
that fixed this oversight. The current logical replication worker design make
it difficult to apply the delay in the catchup phase; tablesync workers are not
closed as soon as the COPY finishes (which means possibly running out of
workers sooner). After all tablesync workers have reached READY state, the
apply delay is activated. The documentation was correct; the code wasn't.
