Skip to content

Ensure workable application health probes#145

Merged
MRiganSUSX merged 4 commits into
DUNE-DAQ:developfrom
jcpunk:probes
Jun 17, 2026
Merged

Ensure workable application health probes#145
MRiganSUSX merged 4 commits into
DUNE-DAQ:developfrom
jcpunk:probes

Conversation

@jcpunk

@jcpunk jcpunk commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

With ERS having hung last week, I've updated the microservices to have trivial naive health probes. They aren't fancy, but should improve recovery under stuck conditions.

@jcpunk
jcpunk requested a review from mroda88 June 9, 2026 17:28

@MRiganSUSX MRiganSUSX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jcpunk.
Here are some comments.

Comment thread opmon-protobuf-dbwriter/dbwriter.py
Comment thread opmon-protobuf-dbwriter/dbwriter.py
Comment thread opmon-protobuf-dbwriter/entrypoint.sh Outdated
Comment thread opmon-protobuf-dbwriter/dbwriter.py Outdated
@MRiganSUSX

Copy link
Copy Markdown
Collaborator

For inspiration, the fixes are applied in https://github.com/DUNE-DAQ/microservices/tree/mrigan/fix_health_probes.

@jcpunk

jcpunk commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Changes merged in

@MRiganSUSX
MRiganSUSX self-requested a review June 16, 2026 14:26

@MRiganSUSX MRiganSUSX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jcpunk for the quick fixes.

I tried test-deploying these in their own namespaces (ers-test and opmon-test).
It did not work out of the box (ofc modding the yamls to target the test env correctly).

After several iterations with AI I ended up with https://github.com/DUNE-DAQ/microservices/tree/fix/health-probes-from-patt, and especially the commit 93aeecd which is fixing 3 things that were problematic. The description has more details:

Fix health server: raw socket accept loop, remove premature shutdown
Three issues fixed in both dbwriters:

1. Replace HTTPServer/serve_forever() with a raw socket accept loop.
   serve_forever() uses EpollSelector which cannot acquire the GIL while
   the Kafka subscriber thread holds it, so accept() is never called and
   probes time out.

2. Remove stop_health_server() from the finally block after sub.start().
   sub.start() is non-blocking — it returns immediately after starting
   Kafka's internal threads, so the finally fired instantly and closed
   the listening socket before any probe arrived.

3. opmon: use influx.ping() instead of influx.get_list_database() in the
   readiness check — lighter weight and purpose-built for health checks.

I'm not an expert, the only thing I can confirm is that it did not work before, and it does work with these changes.

Thoughts on these? Should we merge some/all into your branch?

@MRiganSUSX

Copy link
Copy Markdown
Collaborator

(the test deployments are still there if you want to play)

@jcpunk

jcpunk commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

I think I merged your branch into mine, did I miss something?

@MRiganSUSX

Copy link
Copy Markdown
Collaborator

This is yet another branch. The first one was a code review, this one is a result of actually testing deploying it.

Three issues fixed in both dbwriters:

1. Replace HTTPServer/serve_forever() with a raw socket accept loop.
   serve_forever() uses EpollSelector which cannot acquire the GIL while
   the Kafka subscriber thread holds it, so accept() is never called and
   probes time out.

2. Remove stop_health_server() from the finally block after sub.start().
   sub.start() is non-blocking — it returns immediately after starting
   Kafka's internal threads, so the finally fired instantly and closed
   the listening socket before any probe arrived.

3. opmon: use influx.ping() instead of influx.get_list_database() in the
   readiness check — lighter weight and purpose-built for health checks.
@jcpunk

jcpunk commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Got it, branch merged in.

@MRiganSUSX
MRiganSUSX self-requested a review June 17, 2026 11:16

@MRiganSUSX MRiganSUSX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run a test deployment and worked as expected with probes returning status.

@MRiganSUSX
MRiganSUSX merged commit f97ac74 into DUNE-DAQ:develop Jun 17, 2026
1 of 2 checks passed
@jcpunk
jcpunk deleted the probes branch June 17, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants