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

[extension/filestorage] Copy values returned by Get #11776

Merged

Conversation

swiatekm
Copy link
Contributor

Description:
Byte arrays returned by bbolt are only valid within transactions, and need to be copied if they are to be returned to the caller. See the documentation for Bucket.Get for reference.

In practice, this is a bit difficult to trigger, and I'm not sure what the conditions are exactly, other than a lot of concurrent access. An unrelated test caught it in GHA, and I'm re-enabling it in this change.

Link to tracking Issue:
#11454

Testing:
Re-enabled an unrelated test that originally caught this. I hadn't succeeded in reproducing it consistently, and it seems very hardware-dependent. In general, I don't think we should write tests that depend heavily on bbolt's implementation details.

@swiatekm swiatekm requested a review from a team June 28, 2022 12:14
@swiatekm swiatekm requested a review from djaglowski as a code owner June 28, 2022 12:14
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 28, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: swiatekm-sumo / name: Mikołaj Świątek (5a9a6d8e1231790474cb49c8b05806ad23516015)

@swiatekm swiatekm force-pushed the fix/storagextension/memory-reuse branch from 9954663 to 5a9a6d8 Compare June 28, 2022 12:16
@swiatekm swiatekm force-pushed the fix/storagextension/memory-reuse branch from 5a9a6d8 to 3f65cf7 Compare June 28, 2022 12:29
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

Thank you @swiatekm-sumo!

@djaglowski djaglowski merged commit efedd91 into open-telemetry:main Jun 28, 2022
@swiatekm swiatekm deleted the fix/storagextension/memory-reuse branch June 28, 2022 15:14
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