[PIP] #14395 Making SchemaRegistry implementation configurable

[Aparajita Singh <ap...@gmail.com>]

Hi,
Please review this proposal: https://github.com/apache/pulsar/issues/14395

-- 
Thanks,
Aparajita

Re: [PIP] #14395 Making SchemaRegistry implementation configurable

[Aparajita Singh <ap...@gmail.com>]

Thanks Matteo. This comment was addressed on the
<https://github.com/apache/pulsar/pull/14102#discussion_r812559749> PR
<https://github.com/apache/pulsar/pull/14102#discussion_r812559749>.
Posting the same here as well:







*imo, schema registry should be maintaining this info. given the below
understanding now, we should not rename the `deleteSchema(...)` method or
replace its usages. but we can rename `deleteSchemaStorage(...)` to
`deleteSchemaFromStorage(...)`. is that fine?as per the current code, when
user tries to explicitly delete a schema from the topic, an empty schema is
pushed to the schema storage so that the latest schema is an empty schema.
this preserves the older versions of the schema in the storage and also
maintains metadata about the delete operation. the topic itself remains
unchanged.the schema is deleted from storage only when the topic itself is
deleted.replacing the `deleteSchema(...)` call with
`deleteSchemaStorage(...)` will mean that we will lose the following
metadata:* user who deleted the schema* timestamp at which the schema was
deleted*
Please let me know if there are any more comments on this.


On Wed, 23 Feb 2022 at 08:36, Matteo Merli <mm...@apache.org> wrote:

> Hi Aparajita,
>
> Thanks for the proposal. Indeed the schema registry was meant to be
> pluggable since the beginning although we skipped the actual
> "plugging" part. It would be good to actually see multiple
> implementations there.
>
> I don't see any risk in this proposal and it's a good time to make
> breaking changes to the SchemaRegistry interface since there are not
> (yet) any implementation other than the default one.
>
> > Renaming a few methods in the SchemaRegistryService interface to reflect
> their behavior. The changes are:
> > Rename deleteSchema to putEmptySchema in SchemaRegistryService
>
> My only concern, based on current behavior, is that in some places in
> the code we're calling `deleteSchema()` while actually we should be
> calling `deleteSchemaStorage()` (using the current names). I guess
> that's probably due to the misleading nature of the method names. We
> should double-check these usages to make sure the expected operation
> is applied.
>
> Matteo
>
>
> --
> Matteo Merli
> <mm...@apache.org>
>
> On Mon, Feb 21, 2022 at 7:53 AM Aparajita Singh
> <ap...@gmail.com> wrote:
> >
> > Hi,
> > Please review this proposal:
> https://github.com/apache/pulsar/issues/14395
> >
> > --
> > Thanks,
> > Aparajita
>


-- 
Thanks,
Aparajita

Re: [PIP] #14395 Making SchemaRegistry implementation configurable

[Matteo Merli <mm...@apache.org>]

Hi Aparajita,

Thanks for the proposal. Indeed the schema registry was meant to be
pluggable since the beginning although we skipped the actual
"plugging" part. It would be good to actually see multiple
implementations there.

I don't see any risk in this proposal and it's a good time to make
breaking changes to the SchemaRegistry interface since there are not
(yet) any implementation other than the default one.

> Renaming a few methods in the SchemaRegistryService interface to reflect their behavior. The changes are:
> Rename deleteSchema to putEmptySchema in SchemaRegistryService

My only concern, based on current behavior, is that in some places in
the code we're calling `deleteSchema()` while actually we should be
calling `deleteSchemaStorage()` (using the current names). I guess
that's probably due to the misleading nature of the method names. We
should double-check these usages to make sure the expected operation
is applied.

Matteo


--
Matteo Merli
<mm...@apache.org>

On Mon, Feb 21, 2022 at 7:53 AM Aparajita Singh
<ap...@gmail.com> wrote:
>
> Hi,
> Please review this proposal: https://github.com/apache/pulsar/issues/14395
>
> --
> Thanks,
> Aparajita