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

Scope array consistency to a context. #4730

Closed
wants to merge 1 commit into from

Conversation

davisp
Copy link
Contributor

@davisp davisp commented Feb 15, 2024

Previously this was scoped to a process which breaks the REST server.


TYPE: IMPROVEMENT
DESC: Scope array consistency to a context.

@KiterLuc
Copy link
Contributor

@ypatia says she's unblocked on this work... @davisp is this PR still required?

@davisp
Copy link
Contributor Author

davisp commented Feb 15, 2024

@ypatia says she's unblocked on this work... @davisp is this PR still required?

It wasn't ever required. I just saw the chatter and discovered that the consistency controller looked easy enough to swap to context specific instead of process specific so I took a quick stab at it. As far as I'm concerned, passing the checks proves it is easy enough which is all I intended for this PR.

I'm more than fine closing this if we don't want to include this work in this release cycle which is more than understandable. It was only about 30m of work and more than half of that was reading the code to figure out how it worked to begin with.

@teo-tsirpanis
Copy link
Member

This would slightly reduce the utility of the consistency feature, for the case where multiple contexts are being used.

How about we add instead a config option named like sm.enable_consistency that allows entirely bypassing the consistency controller when opening an array?

@KiterLuc KiterLuc closed this Feb 20, 2024
@davisp
Copy link
Contributor Author

davisp commented Feb 20, 2024

This would slightly reduce the utility of the consistency feature, for the case where multiple contexts are being used.

The problem originated from an issue on cloud where we very much don't want multiple independent contexts interfering with each other. But this was only a learning exercise so not really a big deal either way at the moment. It just might be something we have to look at harder in the future. Adding a config value in the future might be a possibility if we decide the current behavior is useful for something.

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.

3 participants