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

Conversation

@Demogorgon314
Copy link
Member

@Demogorgon314 Demogorgon314 commented Oct 20, 2021

Motivation

When we update Pulsar 2.9.0 dependencies in KoP, there will be some compatibility issues. like:

  • getPolicies API has changed

Modifications

  • Update Pulsar dependencies to 2.9.0
  • Change new getPolicies API

@Demogorgon314 Demogorgon314 added the type/feature Indicates new functionality label Oct 20, 2021
@Demogorgon314 Demogorgon314 self-assigned this Oct 20, 2021
@Demogorgon314 Demogorgon314 marked this pull request as draft October 20, 2021 02:35
@Demogorgon314 Demogorgon314 marked this pull request as ready for review October 20, 2021 07:04
Copy link
Collaborator

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

Could you avoid unnecessary changes as much as possible? Otherwise, it would cause many problems if we want to cherry-pick some PRs to branch-2.8 in future.

For example, I found MetadataStoreCacheLoader was already included in Pulsar 2.8.0. So changing MetadataCache<LocalBrokerData> to MetadataStoreCacheLoader is unnecessary.

We must avoid any change that is not related to the Pulsar upgrade.

@BewareMyPower
Copy link
Collaborator

Pulsar has removed getZkClient in pulsarService

Yeah. But does Pulsar 2.8.1.0 have some alternatives for getZkClient and related APIs? If yes, please open another PR to do that.

Same (Some?) API changes

Similarly, avoid these changes for a Pulsar upgrade PR.

For a PR that upgrades the Pulsar dependency, we should only retain code changes like #746.

@Demogorgon314
Copy link
Member Author

Thank you for pointing out. I'll recheck branch-2.8 and branch-2.9 APIs. And open another PR to fix it.

@Demogorgon314 Demogorgon314 force-pushed the impl/update-pulsar-dependencies-to-2.9.0 branch from 965aa84 to 31936d6 Compare October 22, 2021 01:20
@Demogorgon314 Demogorgon314 changed the title Update Pulsar dependencies to 2.9.0 Bump pulsar version to 2.9.0 Oct 22, 2021
@BewareMyPower
Copy link
Collaborator

We should wait the rc release based on branch-2.9 first.

@Demogorgon314
Copy link
Member Author

I convert this PR to draft now. When 2.9.0 RC released I will update.

@Demogorgon314 Demogorgon314 marked this pull request as draft October 22, 2021 02:47
@Demogorgon314 Demogorgon314 marked this pull request as ready for review October 24, 2021 12:32
@BewareMyPower BewareMyPower merged commit 1704a18 into streamnative:master Oct 25, 2021
BewareMyPower pushed a commit that referenced this pull request Oct 26, 2021
## Motivation
When we update Pulsar 2.9.0 dependencies in KoP, there will be some compatibility issues. like:
* getPolicies API has changed

## Modifications
* Update Pulsar dependencies to 2.9.0
* Change new getPolicies API
BewareMyPower pushed a commit that referenced this pull request Dec 3, 2021
Currently, the KoP store message logic is in `KafkaRequestHandler`, to avoid duplicate code and easier to support idempotent produce, we must refactor the message storage logic.

* Refactor message storage logic.
* The `ReplicaManager` is a public interface to store messages, we should always use `ReplicaManager` to store the message in any situation.

* `PartitionLog` mapping to Kafka is `Log`, the reason for using this name is to avoid naming conflict, like: `log.debug()`.

* `PartitionLogManager` hold all `PartitionLog`, key is full partition name, value is `PartitionLog`.

* Add fetch operation

Fix conflicts made by #819.

The `path` method was removed from master. Keep this method in
branch-2.8.1.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

type/feature Indicates new functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants