Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[transaction] Producer state manager snapshot recovery - Part-1: Add snapshot I/O buffer #1906

Conversation

Demogorgon314
Copy link
Member

@Demogorgon314 Demogorgon314 commented Jun 16, 2023

This PR is a part of the Producer state manager snapshot recovery implementation, it cherry-picks from datastax/starlight-for-kafka#29.

Motivation

Introduce the ProducerStateManagerSnapshotBuffer.
It can write and read the latest snapshot from a specified topic.

This PR introduces two different ProducerStateManagerSnapshotBuffer implementations:

  • PulsarTopicProducerStateManagerSnapshotBuffer: Store the snapshot into the topic.
  • PulsarPartitionedTopicProducerStateManagerSnapshotBuffer : Store the snapshot in a partitioned topic.

Modifications

  • Introduce ProducerStateManagerSnapshotBuffer interface.
  • Provide two diffident implementations for ProducerStateManagerSnapshotBuffer.
  • Add units test to cover the Buffer.

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@Demogorgon314 Demogorgon314 self-assigned this Jun 16, 2023
@github-actions github-actions bot added the no-need-doc This pr does not need any document label Jun 16, 2023
@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #1906 (6ac9e07) into master (08c0a9b) will decrease coverage by 0.41%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1906      +/-   ##
============================================
- Coverage     18.76%   18.35%   -0.41%     
- Complexity      742      743       +1     
============================================
  Files           185      190       +5     
  Lines         13266    13584     +318     
  Branches       1213     1257      +44     
============================================
+ Hits           2490     2494       +4     
- Misses        10594    10909     +315     
+ Partials        182      181       -1     
Impacted Files Coverage Δ
...tive/pulsar/handlers/kop/KafkaProtocolHandler.java 0.00% <0.00%> (ø)
...pulsar/handlers/kop/KafkaTopicConsumerManager.java 0.00% <0.00%> (ø)
...lers/kop/NamespaceBundleOwnershipListenerImpl.java 0.00% <0.00%> (ø)
...ve/pulsar/handlers/kop/TopicEventListenerImpl.java 0.00% <0.00%> (ø)
...ve/pulsar/handlers/kop/TopicOwnershipListener.java 0.00% <0.00%> (ø)
...lers/kop/storage/ProducerStateManagerSnapshot.java 0.00% <0.00%> (ø)
...op/storage/ProducerStateManagerSnapshotBuffer.java 0.00% <0.00%> (ø)
...tionedTopicProducerStateManagerSnapshotBuffer.java 0.00% <0.00%> (ø)
...PulsarTopicProducerStateManagerSnapshotBuffer.java 0.00% <0.00%> (ø)

... and 5 files with indirect coverage changes

gaoran10
gaoran10 previously approved these changes Jun 19, 2023
Copy link
Contributor

@gaoran10 gaoran10 left a comment

Choose a reason for hiding this comment

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

LGTM

@Demogorgon314
Copy link
Member Author

@BewareMyPower Can you help take a look at this PR?

/**
* Use memory to store the latest snapshot.
*/
public class MemoryProducerStateManagerSnapshotBuffer implements ProducerStateManagerSnapshotBuffer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should remove this class and related tests. Because it is only used in tests, which is meaningless.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is a similar example: #1805

There is no bug with MemorySchemaStorage because it's protected by tests. However, it is never used out of tests. PulsarSchemaStorage is actually and always used and it's buggy.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, addressed.

@Demogorgon314 Demogorgon314 merged commit a6eb7a6 into streamnative:master Jun 21, 2023
5 of 7 checks passed
@Demogorgon314 Demogorgon314 deleted the transaction/Support_producer_state_manager_recovery_part-1 branch June 21, 2023 00:28
BewareMyPower pushed a commit that referenced this pull request Jun 29, 2023
…snapshot I/O buffer (#1906)

This PR is a part of the Producer state manager snapshot recovery implementation, it cherry-picks from datastax/starlight-for-kafka#29.

### Motivation

Introduce the `ProducerStateManagerSnapshotBuffer`.
It can write and read the latest snapshot from a specified topic.

This PR introduces two different `ProducerStateManagerSnapshotBuffer` implementations:
* `PulsarTopicProducerStateManagerSnapshotBuffer`: Store the snapshot into the topic.
* `PulsarPartitionedTopicProducerStateManagerSnapshotBuffer` : Store the snapshot in a partitioned topic.

### Modifications

* Introduce `ProducerStateManagerSnapshotBuffer` interface.
* Provide two diffident implementations for `ProducerStateManagerSnapshotBuffer`.
* Add units test to cover the `Buffer`.

(cherry picked from commit a6eb7a6)
BewareMyPower pushed a commit that referenced this pull request Jul 3, 2023
…snapshot I/O buffer (#1906)

This PR is a part of the Producer state manager snapshot recovery implementation, it cherry-picks from datastax/starlight-for-kafka#29.

### Motivation

Introduce the `ProducerStateManagerSnapshotBuffer`.
It can write and read the latest snapshot from a specified topic.

This PR introduces two different `ProducerStateManagerSnapshotBuffer` implementations:
* `PulsarTopicProducerStateManagerSnapshotBuffer`: Store the snapshot into the topic.
* `PulsarPartitionedTopicProducerStateManagerSnapshotBuffer` : Store the snapshot in a partitioned topic.

### Modifications

* Introduce `ProducerStateManagerSnapshotBuffer` interface.
* Provide two diffident implementations for `ProducerStateManagerSnapshotBuffer`.
* Add units test to cover the `Buffer`.

(cherry picked from commit a6eb7a6)
Demogorgon314 added a commit to Demogorgon314/kop that referenced this pull request Aug 14, 2023
…snapshot I/O buffer (streamnative#1906)

This PR is a part of the Producer state manager snapshot recovery implementation, it cherry-picks from datastax/starlight-for-kafka#29.

### Motivation

Introduce the `ProducerStateManagerSnapshotBuffer`.
It can write and read the latest snapshot from a specified topic.

This PR introduces two different `ProducerStateManagerSnapshotBuffer` implementations:
* `PulsarTopicProducerStateManagerSnapshotBuffer`: Store the snapshot into the topic.
* `PulsarPartitionedTopicProducerStateManagerSnapshotBuffer` : Store the snapshot in a partitioned topic.

### Modifications

* Introduce `ProducerStateManagerSnapshotBuffer` interface.
* Provide two diffident implementations for `ProducerStateManagerSnapshotBuffer`.
* Add units test to cover the `Buffer`.

(cherry picked from commit a6eb7a6)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants