Skip to content

[Fix][Connector-V2] Run Pulsar cleanup with connector classloader#11170

Open
DanielLeens wants to merge 4 commits into
apache:devfrom
DanielLeens:dev-pulsar-cleanup-preload-20260623
Open

[Fix][Connector-V2] Run Pulsar cleanup with connector classloader#11170
DanielLeens wants to merge 4 commits into
apache:devfrom
DanielLeens:dev-pulsar-cleanup-preload-20260623

Conversation

@DanielLeens

@DanielLeens DanielLeens commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Purpose

Replace the version-specific Pulsar/Netty/HK2 private-class preload workaround with a maintainable cleanup boundary.

Root cause

Pulsar can lazily resolve shaded Netty/Jersey implementation classes while closing producers, consumers, clients, and admins. SeaTunnel close paths can run from engine threads whose context classloader no longer points to the Pulsar connector, so hard-coding private helper class names only masks the symptom and will break across Pulsar dependency changes.

Changes

  • Run Pulsar cleanup actions with the Pulsar connector classloader as the thread context classloader.
  • Close source split readers before closing the shared Pulsar client.
  • Stop periodic partition discovery before closing the Pulsar admin.
  • Add unit coverage for classloader restoration and source cleanup order.

Validation

  • ./mvnw -pl seatunnel-connectors-v2/connector-pulsar -am -nsu spotless:apply
  • ./mvnw -pl seatunnel-connectors-v2/connector-pulsar -nsu test

seatunnel-engine-ui was not included because this PR only changes the Pulsar connector backend module.

@davidzollo davidzollo marked this pull request as draft June 29, 2026 12:51
@DanielLeens DanielLeens changed the title [Fix][Connector-V2] Preload Pulsar cleanup runtime classes [Fix][Connector-V2] Run Pulsar cleanup with connector classloader Jun 29, 2026
@DanielLeens DanielLeens marked this pull request as ready for review June 29, 2026 13:00
@davidzollo davidzollo force-pushed the dev-pulsar-cleanup-preload-20260623 branch from 1d81699 to 49c46d0 Compare June 29, 2026 14:15

@DanielLeens DanielLeens left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the contribution. I reviewed the latest head from the Pulsar source/sink shutdown paths through the classloader and cleanup-order boundaries, and this looks good to me.

What this PR solves

  • User pain: Pulsar producer / consumer / client / admin cleanup can also run from engine-managed threads. If the thread context classloader no longer points at the Pulsar connector, lazy class resolution during close can fail. On top of that, the previous close order left shared admin/client resources easier to shut down before their users were fully stopped.
  • Fix approach: run Pulsar cleanup through PulsarConfigUtil.runWithConnectorClassLoader(...), stop the discovery executor before admin close, close split readers before the shared client, and preserve all cleanup failures instead of losing later ones.
  • One-line summary: this is a targeted Pulsar shutdown-boundary fix, not a behavior change in normal read/write semantics.

Full execution path I checked

Pulsar task shutdown
  -> source reader / sink writer / committer / enumerator close
      -> producer / consumer / client / admin cleanup
  -> before this PR
      -> wrong TCCL can break lazy class resolution during close
      -> shared admin/client can be closed while readers or discovery still depend on them
  -> after this PR
      -> connector classloader is restored for cleanup
      -> discovery executor stops before admin close
      -> split readers close before shared client close
      -> cleanup failures are aggregated instead of truncating later cleanup

Key findings

  1. The normal runtime path definitely hits this change because all Pulsar close paths now go through the helper.
  2. This PR improves two real lifecycle boundaries at once: classloader context during cleanup, and shutdown ordering of shared resources.
  3. The new tests cover the classloader boundary and the close-time contract on the writer/source side.
  4. I do not see a reopened blocker on the current head.

Findings

  • No blocking issues from Daniel's side on the latest head.

Conclusion: can merge

  1. Blocking items
  • None.
  1. Suggested non-blocking follow-up
  • None.

Overall, this is a precise fix for the Pulsar cleanup boundary and shutdown ordering, and I'm happy to approve it.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant