Miretpl commented on PR #69608:
URL: https://github.com/apache/airflow/pull/69608#issuecomment-4939836902

   I've dug deeper into this, and I think that the analysis is a bit off, as 
PgBouncer exists right after the signal, because the signal is SIGINT and 
PgBouncer closed all its connections to the database quickly enough that it 
seems instantaneous. As far as I looked at the PgBouncer codebase, since 
[1.23.0](https://www.pgbouncer.org/changelog#pgbouncer-123x), there are two 
distinct behaviours of shutting down PgBouncer, and both are good depending on 
the case:
   1. 
[SIGTERM](https://github.com/pgbouncer/pgbouncer/blob/pgbouncer_1_23_0/src/main.c#L488)
 - it waits for closing all of the application connections (Airflow) to 
PgBouncer and shuts down
   2. 
[SIGINT](https://github.com/pgbouncer/pgbouncer/blob/pgbouncer_1_23_0/src/main.c#L505)
 - it closes all PgBouncer connections to the database and shuts down
   
   I would say that the resolution to the issue is removing the `preStop` 
condition and making the transition handled by the load balancer and client 
instead of trying to synchronise it with sleeps, which may vary vastly 
depending on the environment and may be hard to adjust properly. By removing 
`preStop`, we will have something like:
   1. Kubernetes deploys a new PgBouncer pod, which is in a ready state
   2. Kubernetes sends SIGTERM to the old PgBouncer pod, which awaits 
application connections to close
   3. During that time, EndpointSlice has information that this pod is `ready: 
false` and `termination: true`, which should make load balancers not direct 
traffic to it
   4. *'if termination grace period will be high enough'* EndpointSlice will 
have information `serving: false` when the readiness probe starts to fail
   5. Two possible scenarios at the end:
      1. All Airflow connections were closed, and PgBouncer exists on itself
      2. TerminationGracePeriod passes, and Kubernetes kills old PgBouncer pod
   
   If I'm missing something, let me know.
   
   It would be good to test it in a real environment with some reasonable 
amount of executions (unfortunately, I will not be able to do that within a 
week or two probably). @bramhanandlingala @andrew-stein-sp would you be able to 
look at it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to