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

Remove redundant metadata synchronization (#1424) #1426

Merged
merged 1 commit into from
Sep 27, 2021

Conversation

troshko111
Copy link
Contributor

No description provided.

@spencergibb
Copy link
Contributor

Interesting, I assume this is a perf issue?

@troshko111
Copy link
Contributor Author

@spencergibb yeah a user reported contention under load, and I honestly can't see any evidence a thread safe map is required (per contract or per actual usage). LMK what your thoughts are on this.

@spencergibb
Copy link
Contributor

I haven't either and can't guess any future issues

@troshko111
Copy link
Contributor Author

After a conversation with @tcellucci we figured that it should not be a breaking change barring use cases where users mutate the metadata of instances from the registry (a bad idea in general) without explicitly syncing access (an even worse idea). I will merge this but keep an open mind about potential effects, if they prove to be widespread, it may get reverted or put behind a property (sync map by default). For now I don't think it warrants to be behind a prop, it's below my safety threshold.

@troshko111 troshko111 merged commit 4d36f04 into master Sep 27, 2021
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.

None yet

2 participants