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

[controller] Allow AdminOperationSerializer to use lower version to serialize the AdminOperation message #1549

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

minhmo1620
Copy link
Collaborator

@minhmo1620 minhmo1620 commented Feb 21, 2025

[controller] Allow AdminOperationSerializer to use lower version to serialize the AdminOperation message

  • This PR allows AdminTopicConsumptionTask to use the config adminOperationProtocolVersion at AdminTopicMetadata on ZK (could be lower version) as the SOT for serializer to serialize the message and send that writer schema version along with the bytes to deserialize back at consumption stage.

Serialization issues when serializing the object by lower version

This issue is figured during the effort of serialize the object with different schema that it was created with.

The use case that I used:

  • UpdateStore is created with AdminOperation schema v84 (current latest version)
  • I tried to serialize this object with schema v74
    => Even though v84 and v74 is compatible, the serialization failed due to ClassCastException (cannot cast Boolean to Number).

Debug process:

  • The writer will loop through the field inside schema v74 and get the index position, then use that index position to retrieve the value of the field.
image
  • However, comparing to v74, v84 (update store) has one more field (separateRealTimeTopicEnabled) in the middle of the schema

image

=> Then the field and position of the writer schema (v74) is basically not matched anymore (expecting pos 21 to be replicationMetadataVersionID (int), but got writeComputationEnabled (boolean) from v84, causing cast exception java.lang.Boolean cannot be cast to java.lang.Number

image

There are two options:

Option 1: Override getField to get the field by fieldName
Option 2: Turn the current AdminOperation (with latest version schema) to GenericRecord (with writer schema id) and serialize the GenericRecord with writer schema id.

Option 1 was implemented (PR) but then closed after getting the suggestion for option 2.

Option 2, though very "bulky", it works and cut the future maintenance cost since we were modifying Avro API. We chose this option over option 1 and the implementation can be found specifically here.

How was this PR tested?

UTs and integration tests

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

@minhmo1620 minhmo1620 changed the title [controller] Update AdminOperationSerializer to use lower version to serialize the AdminOperation message [controller] Allow AdminOperationSerializer to use lower version to serialize the AdminOperation message Feb 21, 2025
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.

1 participant