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

GlobalId Strategy should seed the Schema cache if it registers a schema #1148

Closed
EricWittmann opened this issue Jan 15, 2021 · 3 comments
Closed
Assignees

Comments

@EricWittmann
Copy link
Member

Some global id strategies can register a new schema with the registry. When they do this they return the globalid of the new artifact/version they created. The Serializer then immediately uses that ID to fetch the schema from the registry (regardless of whether it was just uploaded or was already registered) and caches it. This is problematic for asynchronous storages like the Streams storage because the Register-then-immediately-fetch workflow fails. We've temporarily added a retry() to the fetch part of this flow, but a better solution is for the register() part to seed the cache so that the fetch doesn't happen.

@EricWittmann EricWittmann self-assigned this Jan 15, 2021
@EricWittmann
Copy link
Member Author

The context of this issue is here:

https://github.com/Apicurio/apicurio-registry/blob/master/utils/serde/src/main/java/io/apicurio/registry/utils/serde/AbstractKafkaSerializer.java#L94-L99

image

Possibly we need to pass the cache to the strategy. Or perhaps we need to rethink the strategies and serdes flow entirely.

@famartinrh - if you had any thoughts here is a good place for them. :)

@alesj alesj self-assigned this Jan 27, 2021
alesj added a commit to alesj/apicurio-registry that referenced this issue Jan 27, 2021
EricWittmann pushed a commit that referenced this issue Jan 27, 2021
* GlobalId Strategy should seed the Schema cache if it registers a schema #1148

* Simplify Streams storage, no need for async global id mapping.
carlesarnal pushed a commit that referenced this issue Feb 2, 2021
* GlobalId Strategy should seed the Schema cache if it registers a schema #1148

* Simplify Streams storage, no need for async global id mapping.
carlesarnal pushed a commit that referenced this issue Feb 15, 2021
* GlobalId Strategy should seed the Schema cache if it registers a schema #1148

* Simplify Streams storage, no need for async global id mapping.
EricWittmann added a commit that referenced this issue Mar 1, 2021
* JAX-RS generated classes for v2 of the Registry API.

* Lots of refactoring to start implementing v2 of the rest api

* JAX-RS generated classes for v2 of the Registry API.

* Added groupId concept to the storage layer

* JAX-RS generated classes for v2 of the Registry API.

* Update ArtifactgroupsResource.java

* Update ArtifactMetaData.java

* Update EditableMetaData.java

* Update SearchedVersion.java

* Update VersionMetaData.java

* Updated the REST API with some recent changes

* Handle serializer cache, simplify Streams storage (#1163)

* GlobalId Strategy should seed the Schema cache if it registers a schema #1148

* Simplify Streams storage, no need for async global id mapping.

* Updated the asyncmem storage

* JAX-RS generated classes for v2 of the Registry API.

* Regenerated the jax-rs layer after modifications to the v2 api design

* JAX-RS generated classes for v2 of the Registry API.

* Additional changes to the v2 REST api

* added jax-rs test for the /groups v2 api endpoint

* V2 api auth support (#1172)

* Add security paths for v2 api endpoints

* Remove localhost placeholder

* Simplify permission paths

* Fixed a failing unit test

* Added a test for multiple groups.

* JAX-RS generated classes for v2 of the Registry API.

* Added more tests and updated the in-memory storage to support contentId and contentHash

* JAX-RS generated classes for v2 of the Registry API.

* Implemented the /ids API endpoints and created tests for them

* minor change to the auth test and its superclass

* updated the asyncmem and ispn storages with latest changes

* removed an unused interface in the UI service

* Created a search REST test

* Stop polluting compatibility layer with REST beans

* Added UI support for groups in the Artifacts page and Upload Artifact modal

* Added UI support for groups.

* Rest client using java 11 http client. (#1195)

* Plain java rest client initial implementation

* Extract client creation into a factory

* Add json body handler

* Use json body handler

* Use entry set

* Extract methods to request handler

* SQL storage - artifact groups (#1189)

* sql storage - artifact groups

* undo unneeded test changes

* searchArtifacts by groupId - make exact search

* add groupId test

* minor code readability changes in the new rest client impl

* Implemented the /ids operations in the new rest client

* Fix testCorrectGroup test

* fixed a test and disabled the auth test when running on windows

* Search artifacts and simplified request handler (#1202)

* Fix accept and content type headers

* Change artifact name

* Artifact search

* Improve rest client code format

* Simplify request handler

* Add search versions

* Fix list query params

* Updated the maven plugins to use the v2 api.  Also updated how the mojos are configured

* Java 11 rest client 1173 (#1204)

* Fix format issues

* Add admin resources to rest client (#1206)

* Use io utils when dealing with streams

* Add artifact rule tests

* Add admin resources to rest client

* Implement admin resources

* Add test comments

* Add auth capabilities

* Refactor request handler and add initial auth capabilities

* Fix empty response expected issue

* Add request headers customization capabilities (#1210)

* Add request headers customization capability

* Improve json error handling (#1211)

* v2 api - dynamic log configuration (#1190)

* api v2 dynamic log configuration impl

* dynamic log configuration: sql storage

* Simplify java 11 rest client (#1212)

* Fix log operations path and enable tests (#1213)

* Fixed a unit test and removed an unused import

* fixed the test URL for two mojo tests

* New integration-tests + serdes 2.0 (#1218)

* serdes refactoring - serdes 2.0

* serde 2.0 - improvements

* improve serdes and add serdes QuarkusTests

* WIP: integration-tests 2.0

* serdes-2.0 impl improvements

* new client bug fixes

* new integration-tests

* test fixes

* fix tests deps in storages

* fix sql storage bug

* fix loggers tests

* fix listArtifactsInGroup offset and limit parameters

* disable infinispan tests

* build integration tests common

* fix legacy tests deps issue

* re-implement more integration-tests

* fix TestUpdateRegistryMojoTest

* Regenerate JAX-RS layer with addition of quarkus annotation on beans (#1223)

* removed some usages of "var" instead of specific var types

* JAX-RS generated classes for v2 of the Registry API.

* Update RuleViolationError.java

* CLI - use v2 api client, use quarkus command mode and compile the tool to native (#1219)

* Add tls and custom configuration support (#1222)

* Add tls and custom configuration support

* Fix compilation error in serdes

* Fix bad param value

* Feat/artifact groups test old serdes release (#1227)

* revert #1132 in v1 API for compatibility with old client

* legacy-tests test old client version(1.3.2.Final) with v1 api

* fix testLoggersCRUD test (#1228)

* JAX-RS generated classes for v2 of the Registry API.

* Update RuleViolationError.java

* Maven build cleanup, removed deprecated v1 client and serdes, disabled connector temporarily

* refactor converters to use serdes 2.0

* Uncomment converter and converter distro

* updated kafka+sql storage to support groups and log configs

* Fixed some bugs in the kafkasql groupId support.

* Fix potential mbean registration error in kafkasql

* Minor improvements found when updating apicurio-registry-examples

* re-organized the API endpoints.  Fixes #1238

* Created an API listing page located at /apis - now all supported APIs are documented there

* Back to using the client to delete global rules for all tests.  Set client to always use HTTP/1

* Adapt auth paths to new api structure (#1255)

* Serdes improvements: config classes, protobuf textual schema support, headers handler, more tests, schema resolver api, deserializers, ... (#1256)

* separated out schema resolver functionality to provide a nice base class for custom resolvers

* Incorrect transitive compatibility checking for JSON schemas. Fixes #1239 (#1257)

* compatibility request filter for the old /api/ path (#1258)

* Support "default" groupId in v2 to simulate "no group" and provide compatibility with v1 api

* Support for "default" groupId in v2 API (#1259)

* Support "default" groupId in v2 to simulate "no group" and provide compatibility with v1 api

* adapt tests to default groupId

Co-authored-by: Fabian Martinez <[email protected]>

* Add artifact groups streams storage 1180 (#1264)

* Change artifact id by artifact key

* Add artifact key support to streams storage layer

* Add artifact key serdes

* Partially fix artifacts search

* Fix metadata search

* Add fixme search metadata

* Add groups and content segregation to streams storage

* Fix update state method call

* Fix content id not being returned in metadata

* Enable streams storage variant tests

* Wip ~ test cluster it with streams

* Change client response from class to type reference

* Remove additional version

* Fix tabs usage

* fix GH workflows

* fix tests

* remove cache dependencies action and run all tests on master branch

* Fix invalid schema integration test for now

* disable ISPN build

* disable infinispan integration tests

Co-authored-by: Aleš Justin <[email protected]>
Co-authored-by: Carles Arnal <[email protected]>
Co-authored-by: Fabian Martinez <[email protected]>
Co-authored-by: Fabian Martinez <[email protected]>
Co-authored-by: Jakub Senko <[email protected]>
Co-authored-by: Carles Arnal <[email protected]>
@EricWittmann EricWittmann assigned famarting and unassigned alesj and EricWittmann Mar 1, 2021
@EricWittmann
Copy link
Member Author

This is resolved in 2.0 with the serdes update, right @famartinrh ?

@EricWittmann
Copy link
Member Author

I think it is, so I'm closing it as resolved. If I'm wrong, @famartinrh can reopen it. :)

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

No branches or pull requests

3 participants