Skip to content

[feat] PIP-454: Metadata Store Migration Framework (implementation)#25219

Open
merlimat wants to merge 7 commits intoapache:masterfrom
merlimat:pip-454-impl
Open

[feat] PIP-454: Metadata Store Migration Framework (implementation)#25219
merlimat wants to merge 7 commits intoapache:masterfrom
merlimat:pip-454-impl

Conversation

@merlimat
Copy link
Contributor

@merlimat merlimat commented Feb 5, 2026

Motivation

Implementation of PIP-454

Modifications

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:

@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Feb 5, 2026
Comment on lines +69 to +110
/**
* Start the migration process.
*
* @throws Exception if migration fails
*/
public void startMigration() throws Exception {
log.info("=== Starting Migration ===");
log.info("Source: {} (current)", sourceStore.getClass().getSimpleName());
log.info("Target: {}", targetUrl);

try {
// 1. Create migration flag
setInitialMigrationPhase();

// 2. Wait for participants to prepare
waitForPreparation();

// 3. Copy persistent data
updatePhase(MigrationPhase.COPYING);
copyPersistentData();

// 4. Set state to completed
updatePhase(MigrationPhase.COMPLETED);

log.info("=== Migration Complete ===");
} catch (Exception e) {
log.error("Migration failed", e);
updatePhase(MigrationPhase.FAILED);
throw e;
}
}

private void setInitialMigrationPhase() throws MetadataStoreException {
try {
sourceStore.put(MigrationState.MIGRATION_FLAG_PATH,
ObjectMapperFactory.getMapper().writer()
.writeValueAsBytes(new MigrationState(MigrationPhase.PREPARATION, targetUrl)),
Optional.of(-1L)).get();
} catch (Exception e) {
throw new MetadataStoreException(e);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is there a solution that prevents starting a new migration unless there's no previous migration? I guess retrying would be allowed so the previous state could be failed. Is there a way to do a CAS update to ensure that only a single node becomes the coordinator?

private void readCurrentState() throws MetadataStoreException {
try {
// Read the current state
var initialState = migrationStateCache.get(MigrationState.MIGRATION_FLAG_PATH).get();
Copy link
Member

Choose a reason for hiding this comment

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

since this is read only at startup time, I guess it could by pass the cache.
In addition, it might be necessary to read after using sync on Zookeeper to ensure that it's the most recent state.
I guess it would be a small chance that this could cause actual issues.

Comment on lines +104 to +108
readCurrentState();
registerAsParticipant();

// Watch for migration events
watchForMigrationEvents();
Copy link
Member

Choose a reason for hiding this comment

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

is there a chance that we lose an event between reading the current state and before the listener is registered?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-required Your PR changes impact docs and you will update later. PIP ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants