Skip to content

[server][dvc][cdc] Fix PubSubPosition serialization helper to include typeId #1978

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

Merged

Conversation

sushantmane
Copy link
Collaborator

Fix PubSubPosition serialization to include both typeId and value payload

Previously, only the raw bytes of the position were being serialized, omitting the type ID
and producing an incomplete PubSubPositionWireFormat. This caused deserialization failures
when attempting to reconstruct the full position.

This change ensures that the full PubSubPositionWireFormat—including the type ID and raw
bytes—is serialized and used consistently during conversion.

Code changes

  • Added new code behind a config. If so list the config names and their default values in the PR description.
  • Introduced new log lines.
    • Confirmed if logs need to be rate limited to avoid excessive logging.

Concurrency-Specific Checks

Both reviewer and PR author to verify

  • Code has no race conditions or thread safety issues.
  • Proper synchronization mechanisms (e.g., synchronized, RWLock) are used where needed.
  • No blocking calls inside critical sections that could lead to deadlocks or performance degradation.
  • Verified thread-safe collections are used (e.g., ConcurrentHashMap, CopyOnWriteArrayList).
  • Validated proper exception handling in multi-threaded code to avoid silent thread termination.

How was this PR tested?

  • New unit tests added.
  • New integration tests added.
  • Modified or extended existing tests.
  • Verified backward compatibility (if applicable).

Does this PR introduce any user-facing or breaking changes?

  • No. You can skip the rest of this section.
  • Yes. Clearly explain the behavior change and its impact.

Previously, only the raw bytes of the position were being serialized, omitting the type ID
and producing an incomplete PubSubPositionWireFormat. This caused deserialization failures
when attempting to reconstruct the full position.

This change ensures that the full PubSubPositionWireFormat—including the type ID and raw
bytes—is serialized and used consistently during conversion.
@sushantmane sushantmane force-pushed the li-fix-bug-in-position-wire-format-ser branch from 33061f3 to e9bda9a Compare July 31, 2025 18:35
@sushantmane sushantmane changed the title [server][dvc][cdc] Fix PubSubPosition serialization to include both typeId and value payload [server][dvc][cdc] Fix PubSubPosition serialization helper to include typeId Jul 31, 2025
@sushantmane sushantmane enabled auto-merge (squash) July 31, 2025 20:42
Copy link
Contributor

@sixpluszero sixpluszero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Thank you for the quick fix!

@sushantmane
Copy link
Collaborator Author

Thanks a lot, @sixpluszero!

@sushantmane sushantmane merged commit 1813bbd into linkedin:main Jul 31, 2025
58 checks passed
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.

2 participants