FasterXML / jackson-core

Core part of Jackson that defines Streaming API as well as basic shared abstractions
Apache License 2.0
2.25k stars 773 forks source link

add volatile flag to SerializedString byte[] fields #1276

Closed jaredstehler closed 3 months ago

jaredstehler commented 4 months ago

Fixes https://github.com/FasterXML/jackson-core/issues/1274

cowtowncoder commented 4 months ago

Hi @jaredstehler! Have you been able to observe the different wrt old code, one with the fix? I think you mentioned that the initial results seemed encouraging, just wanted to follow up.

If this fix needs to go in (which it sounds it does), it probably has negative performance effect so I want to try to make sure it does resolve the problem observed.

jaredstehler commented 4 months ago

I ran with this fork in our environment for an extended test, and did not observe the issue for > 14 days. Subsequently, when rolling out the fork more fully after the test, we observed the issue during the time when a service had switched back to mainline 2.12.6.

I will continue to monitor and report back if we observe the issue for any service running our fork, which would rule out the fix, but our findings do seem to line up with this being the culprit.

cowtowncoder commented 4 months ago

@jaredstehler Sounds good. I will off merging this for a bit now, given that there is plenty of time to get this in 2.18, and I don't intend to backport this in 2.17 (but even if I did, 2.17.1 was just released so 2.17.2 will probably take 2-4 months to release anyway).

cowtowncoder commented 3 months ago

@jaredstehler For some reason I was unable to make changes to this PR so used it as a base for #1297 to update release notes etc. Regardless, now merged in 2.18 for inclusion in 2.18.0. Thank you!