Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add volatile flag to SerializedString byte[] fields #1276

Open
wants to merge 1 commit into
base: 2.18
Choose a base branch
from

Conversation

jaredstehler
Copy link

Fixes #1274

@cowtowncoder
Copy link
Member

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
Copy link
Author

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
Copy link
Member

@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).

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.

NUL-corrupted keys, values on JSON serialization
2 participants