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

Exclusive WriteableRegistry for plugins/modules/libraries #7840

Open
Tracked by #8797
parasjain1 opened this issue May 31, 2023 · 3 comments
Open
Tracked by #8797

Exclusive WriteableRegistry for plugins/modules/libraries #7840

parasjain1 opened this issue May 31, 2023 · 3 comments
Labels
discuss Issues intended to help drive brainstorming and decision making enhancement Enhancement or improvement to existing feature or request Plugins

Comments

@parasjain1
Copy link
Contributor

parasjain1 commented May 31, 2023

Is your feature request related to a problem? Please describe.
In #7780 we've introduced support for opensearch modules, plugins or libraries to be able to register their own writer and reader implementations.

There's a concern with ensuring that the class identifier byte be kept unique across all such implementations.

WriteableRegistry::registerReader() will throw an exception if the ordinal byte used to register the reader already exists for some other reader in the READER_REGISTRY, but we could still register a reader using the same byte as for JodaCompatibleZonedDateTime from within a static block in Security Plugin and successfully execute the security plugin unit tests. This shall fail during runtime though.

Describe the solution you'd like

Have external modules/plugins/libraries to optionally maintain their own WriteableRegistry to avoid type ID collision when using writeGenericValue and readGenericValue. Reference to this registry can either be passed to StreamInput/StreamOutput constructors or to the writeGenericValue, readGenericValue methods. When passed, the StreamInput/StreamOutput will use this registry instead of the "core" one.

As of now BaseWriteable.WriteableRegistry is just a utility class with static maps that hold readers/writers. This will need refactoring to support creation of registry objects.

Describe alternatives you've considered
Reserving exclusive offsets or byte ranges for each plugin/module type IDs - This won't scale well as we reserve one byte for type ID with 256 possible values only. Further, it'd be counterintuitive wrt the idea of decoupling core from plugins/modules as we'd have to maintain the offset/range information for plugins within core.

relates to #7780
relates to #5910
relates to #7821

@parasjain1 parasjain1 added enhancement Enhancement or improvement to existing feature or request untriaged labels May 31, 2023
@parasjain1 parasjain1 changed the title Pluggable WriteableRegistry Exclusive WriteableRegistries for plugins/modules/libraries May 31, 2023
@parasjain1 parasjain1 changed the title Exclusive WriteableRegistries for plugins/modules/libraries Exclusive WriteableRegistry for plugins/modules/libraries May 31, 2023
@minalsha minalsha added discuss Issues intended to help drive brainstorming and decision making and removed untriaged labels Jul 14, 2023
@minalsha
Copy link
Contributor

@nknize @dblock any thought on this enhancement request?

@nknize
Copy link
Collaborator

nknize commented Jul 15, 2023

I think we should look at converging on NamedWriteableRegistry by adding Writer support and get away from trappy read/writeGeneric.

@nknize
Copy link
Collaborator

nknize commented Jul 20, 2023

@minalsha I opened a meta issue where I'll track my prerequisite changes that need to merge in order to cut over to the NamedWriteableRegistry from the current Streamables registry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues intended to help drive brainstorming and decision making enhancement Enhancement or improvement to existing feature or request Plugins
Projects
None yet
Development

No branches or pull requests

4 participants