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

[fix][broker] Fix avoid creating new topic after migration is started #21368

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

rdhabalia
Copy link
Contributor

Motivation

Right now, blue broker deletes the topic once all producers and subscriptions are migrated to green cluster. Blue cluster persist topic's migration state into zk with managed-ledger metadata. However, once topic fully migrates to green cluster, blue cluster broker deletes the topic and deletes the topic's managed-ledger metadata because of that next time if client connects to blue-broker on the same topic, blue broker recreates the same topic again which can cause incorrect connection and data loss during migration. Therefore, blue broker should not allow any new topic creation if migration is started for that namespace to avoid such data loss and incorrect connection. This PR depends on #21354

Modifications

Broker fails to create topic if migration is enabled for that namespace and sends migration error to client so, client can connect to green cluster.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

…apache#584)

* [fix][broker] Fix avoid creating new topic after migration is started

fix migration

* add org.apache.pulsar.broker.service.ClusterMigrationTest test

* fix test

Co-authored-by: rdhabalia <rdhabalia@apache.org>

clean up file

fix formatting
@rdhabalia rdhabalia added area/broker doc-not-needed Your PR changes do not impact docs ready-to-test labels Oct 16, 2023
@rdhabalia rdhabalia self-assigned this Oct 16, 2023
Copy link
Contributor

@vineeth1995 vineeth1995 left a comment

Choose a reason for hiding this comment

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

Looks good

@rdhabalia rdhabalia merged commit e088411 into apache:master Oct 16, 2023
43 of 45 checks passed
@rdhabalia rdhabalia deleted the new_topic_mig2 branch October 16, 2023 18:26
vraulji567 pushed a commit to vraulji567/pulsar that referenced this pull request Oct 16, 2023
@Technoboy- Technoboy- added this to the 3.2.0 milestone Oct 22, 2023
@Technoboy-
Copy link
Contributor

Hi @rdhabalia , cluster migration is introduced by 3.0, so I think we need to cherry-pick this patch to branch-3.0 and branch-3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants