MDC properties inconsistency in health checks after recent update

[atanasnik]

Hello,

We have recently updated a project to the LTS (3.20.0). To summarise, the distribution of MDC properties across health check logs seems non-deterministic. As this has happened before, here is a link to the previous discussion where the topic is explained in detail (the issue was resolved). I cannot reproduce the problem when switching to 3.12.3, for example, as this was the version where the problem was fixed.

I am not sure whether the same problem has reoccurred, or if anything else has happened, that is leading to this inconsistency.
Any advice is appreciated.

[quarkus-bot[bot]]

/cc @jmartisk (health), @xstefank (health)

[xstefank]

Hi @atanasnik, that's strange, nothing change in this code. I will double-check the original reproducer but I'm also working on a major update of SR Health that might help with this too.

[xstefank]

@atanasnik I updated my reproducer to the latest Quarkus 3.21.4 - https://github.com/xstefank/quarkus-reproducers/tree/main/mdc-propagation and it still seems to work.

I tried also with 3.20.0 and I also can't reproduce it.

EDIT: I also added a test that calls all health check 100000 times and it passes - https://github.com/xstefank/quarkus-reproducers/blob/main/mdc-propagation/src/test/java/org/acme/MDCLoggingTest.java.

Can you reproduce it all the time?

[atanasnik]

Hi, thank you for reaching out! I tried to reproduce the problem here: xstefank/quarkus-reproducers#2
It seems to fail only when adding the MDC.put in the mentioned endpoint, in this case.

[xstefank]

Hi @atanasnik, unfotunatelly, I can't figure out what MDC does to mess up Vertx context that is otherwise provided by Quarkus. But I was able to fix your use case by overriding the QuarkusAsyncHealtchCheckFactory to use a new vertx instance - xstefank/quarkus-reproducers@fddaf23.

I must say that I don't think this is a correct solution but I don't understand VertxMDC enough to be sure that any changes that I make won't break other behaviours which are now tight to threads. Let me know if this will work for you or not please.

[cescoffier]

Do you have the 2 code snippets?

[atanasnik]

I used the code from the last commit provided in this discussion, the only thing I did was to replace line 38 with Context duplicatedContext = VertxContext.newNestedContext().

[cescoffier]

As I said in the other thread, you should never call Vertx.vertx() yourself.

[atanasnik]

Yes, I understand, though I am not sure what we can do next.

[cescoffier]

Remove that call and retrieve the current duplicated context—make sure you stay on this one and nothing creates a new one.

[xstefank]

@atanasnik can you try this PR in your application please? #48009

[atanasnik]

Hi! I cannot guarantee this 100%, but I am currently unable to reproduce the problem, after building with this PR's changes.

[xstefank]

@atanasnik, the PR was rejected for valid reasons, and I closed it. But in the end, I think I found out what the issue is. If you can give it another spin with this new PR, I would like to have your ack -> #48057.

[xstefank]

PR is merged so please mark this as an answer if it is solved for you. Thanks.

[atanasnik]

Thank you! I tried quarkus 3.23.0 but I can still reproduce the problem locally, as well as with the mdc-propagation reproducer project. :(

[gsmet]

That's expected, the fix will be part of 3.23.1 that should be released today.

[cescoffier]

We found the explanation for the inconsistency. The fix will still work, but it may not be required once the root cause is fixed.

[atanasnik]

Hello! I was now able to see the discussion on #48678. I just wanted to check if the problem we discussed in the current issue is considered resolved as of the latest Quarkus version, or if other related problems need to be addressed first for this one to be considered finalized. Thank you!

[cescoffier]

We fixed the broken Vert.x duplicated context behavior.

[atanasnik]

Should we close this discussion?