Skip to content

[Fix-18640][Registry] Preserve previous values in etcd REMOVE events - #18641

Open
michaellx1057 wants to merge 3 commits into
apache:devfrom
michaellx1057:Fix-18640
Open

michaellx1057 wants to merge 3 commits into
apache:devfrom
michaellx1057:Fix-18640

Conversation

@michaellx1057

@michaellx1057 michaellx1057 commented Sep 15, 2026

Copy link
Copy Markdown

Was this PR generated or assisted by AI?

YES. Codex generated the initial Java patch and regression tests, assisted
with source analysis and this description, and ran the validation commands
reported below. AI review and automated test results do not replace maintainer review.

是。Codex 生成了初始 Java 补丁和回归测试,协助源码分析与本文说明,
并执行了下方记录的验证命令。
AI 复核和自动化测试结果不替代维护者审阅。

Purpose of the pull request

Fixes #18640.

Etcd DELETE watch events carry the removed value in prevKV, but the
registry converter currently reads the empty current value. Cluster
subscribers can then ignore REMOVE and retain departed members.

This change restores the REMOVE event-data contract. It does not change
leader-election logic or claim to fix all duplicate workflow executions.

etcd DELETE 的旧值在 prevKV 中,但现有转换器读取当前空值,导致成员
监听器忽略 REMOVE。此修复恢复事件数据合同,不修改选主逻辑。

Brief change log

  • Select the previous KV for REMOVE directly in eventData; use the current KV for other events.
  • Continue deriving the event path from the current KV.
  • Preserve ADD and UPDATE behavior.
  • Add deterministic etcd conversion regression tests.
  • Add a shared event-data integration test for etcd, ZooKeeper, MySQL, and
    PostgreSQL while preserving the existing subscription test.

中文:DELETE 使用旧值,路径继续使用当前 key,保持 ADD/UPDATE 行为,
并增加 etcd 专项回归测试及四后端共用事件内容测试;保留原订阅用例。
按审阅建议内联值选择,移除新增的 Event 字段注释。

Verify this pull request

Validated against dev commit
9839c418c1a6d7f2a8c3395552edcd99c2de1b37 with Zulu JDK 8.

  • Before the fix: 4 regression tests, with only DELETE previous-value delivery
    failing (expected previous-heartbeat, actual empty string).
  • After the fix: all 4 regression tests passed.
  • The new shared testSubscribeEventData also fails against the original
    etcd converter: REMOVE expects v2 but receives an empty string.
  • Previous revision (bd24836), clean run: 61 tests passed, 0 failures, errors, or skipped tests:
    14 each for etcd, ZooKeeper, MySQL, and PostgreSQL, plus 4 etcd conversion
    tests and 1 lease-manager test.
  • After the review simplification, a fresh JDK 8 clean run passed all 19 etcd tests:
    14 registry integration tests, 4 event conversion tests, and 1 lease-manager test,
    with 0 failures, errors, or skipped tests.
  • Spotless apply/check and git diff --check passed. The final clean reactor
    run kept the default Spotless and JaCoCo checks enabled.

Reproduction command (JDK 8 and Docker required):

./mvnw -B -ntp \
  -pl dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-etcd,dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-zookeeper,dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc \
  -am clean test \
  -Dtest=EtcdRegistryEventTest,EtcdRegistryTestCase,EtcdKeepAliveLeaseManagerTest,ZookeeperRegistryTestCase,MysqlJdbcRegistryTestCase,PostgresqlJdbcRegistryTestCase \
  -Dsurefire.failIfNoSpecifiedTests=false

The local Apple Silicon run used the existing test image
ibmcom/etcd:3.2.24 with DOCKER_DEFAULT_PLATFORM=linux/amd64 and temporary
Maven Central mirror settings. No repository build configuration was changed.

The JDBC test fork emitted a shutdown-timeout diagnostic after its tests
passed. A control run using the unchanged shared test emitted the same
diagnostic; both builds completed successfully with exit code 0.

中文:修复前 DELETE 旧值测试失败,其余 3 项通过;修复后全部通过。
新增共用用例在旧 etcd 转换器下也能检出 REMOVE 数据为空。
此前版本 bd24836 的 JDK 8 clean 构建共 61 项测试通过,无失败、错误或跳过;格式与差异检查通过。
本次按审阅意见精简后,重新执行 JDK 8 clean 构建:19 项 etcd 测试全部通过,无失败、错误或跳过。
上述本地测试覆盖事件转换及四后端注册、锁和 etcd 租约,不作为多 Master
端到端验收或生产部署的证明。

Pull Request Notice

Pull Request Notice

This change does not modify the Registry public API, database schema,
dependencies, or configuration. It corrects the value delivered for etcd
REMOVE events.

本次不修改 Registry 公共 API、数据库结构、依赖或配置,仅纠正 etcd
REMOVE 事件携带的值。

@boring-cyborg

boring-cyborg Bot commented Sep 15, 2026

Copy link
Copy Markdown

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

@ruanwenjun ruanwenjun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ruanwenjun ruanwenjun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Read removed values from prevKV while retaining the current event key.
Add regression tests for DELETE, ADD and UPDATE event conversion.

Generated-by: Codex
Add a shared subscription payload test while preserving the existing test.
Check current ADD/UPDATE values, last REMOVE value, and event paths.
Clarify Event data semantics without changing the public API.

Generated-by: Codex
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.

[Bug] [Registry] Etcd DELETE events lose previous values and leave stale members

3 participants