NeverENG opened a new pull request, #3690:
URL: https://github.com/apache/dubbo-go/pull/3690
## What
Fixes #3459 — deprecates the fork `github.com/dubbogo/go-zookeeper` and
switches to upstream `github.com/go-zookeeper/zk v1.0.4`.
**⚠️ Blocked by the companion gost PR dubbogo/gost#147**: gost's
`database/kv/zk` wrapper must be migrated first because it depended on the
fork's private watcher API (`*Watcher.EvtCh`) and importable test helpers.
Until gost cuts a release, this PR carries a temporary `replace
github.com/dubbogo/gost => <migrated gost>` directive (marked `TODO(#3459)` in
go.mod); it will be swapped for the released version before merge, which will
also restore the gost checksum lines in go.sum.
## Changes
The dubbo-go side turned out to be small, exactly as the issue analysis
predicted — most call sites only use error variables (`ErrNoNode`,
`ErrNodeExists`, `ErrBadVersion`), `Event`/`State` constants, and `Stat`, all
of which are identical upstream:
- One-line import swap (`github.com/dubbogo/go-zookeeper/zk` →
`github.com/go-zookeeper/zk`) in:
- `remoting/zookeeper/listener.go`
- `remoting/zookeeper/curator_discovery/service_discovery.go`
- `config_center/zookeeper/impl.go`
- `registry/zookeeper/registry.go`
- `metadata/report/zookeeper/report.go`, `report_test.go`
- Direct `Conn.ExistsW` call sites already discarded the third return value,
so the `*Watcher` → `<-chan Event` change compiles without code changes.
- `config_center/zookeeper/impl_test.go`: adapted to gost's new
`NewMockZookeeperClient` signature (3 return values; the `*zk.TestCluster`
return no longer exists since upstream keeps test-cluster helpers in `_test.go`
files). The tests now run against a real ZooKeeper (`ZK_ADDR`, default
`127.0.0.1:2181`) and skip cleanly when none is reachable.
- `go.mod`/`go.sum`: drop `dubbogo/go-zookeeper`, add
`github.com/go-zookeeper/zk v1.0.4`, temporary gost replace, `go mod tidy`.
- `tools/dubbogo-cli` is a separate module pinned to a published dubbo-go
release, so it is intentionally untouched; its scaffold templates can move
after this migration ships in a release.
## Verification — the regression checklist from #3459, all against a real
ZooKeeper 3.9.5
| Checklist item | Result |
|---|---|
| Registry register / subscribe / unsubscribe | ✅ live test: temp-node
registration, add/delete child events received while subscribed, zero callbacks
after `Close()` (5s observation window), no goroutine leak |
| Config Center watch & change notification | ✅
`TestPublishAndRemoveConfigWithMockZk`, `TestGetPropertiesWithMockZk` and
listener tests pass against a live server |
| Metadata Report service-mapping watch | ✅ all 21 tests pass against a live
server, incl. mapping register/listen/remove |
| **Watcher re-establishment after disconnect/reconnect** | ✅ see below |
| Unit tests | ✅ `go build ./...` clean; the 4 zookeeper-related packages:
**47 passed / 0 skipped / 0 failed** with a live server; gost `database/kv/zk`:
8/8 |
### Disconnect/reconnect detail (the risk point called out in the issue)
- **Session expiry (strict case)**: server stopped for 15s with a 4s session
timeout, then restarted. Client re-established the session in ~0.3s; the watch
was re-armed and delivered add/delete events within 30ms of reconnect. This
works because upstream delivers `EventNotWatching` to all watchers on session
expiry, which unblocks `listenDirEvent`'s loop to re-call `GetChildrenW` on the
new session — the existing re-watch loop in `remoting/zookeeper/listener.go`
needs no changes.
- **Transient blip, session survives** (server paused 2s via
SIGSTOP/SIGCONT): the server restored watches transparently via `set-watches`;
no re-subscribe path was needed and events kept flowing.
Two pre-existing behaviors were observed and confirmed to predate this
migration (identical code path before/after): duplicate `Add` events from
`handleZkNodeEvent`'s full-list diff, and an occasional benign `zk: node does
not exist` error log when a freshly-created child is deleted within the
enumeration race window.
## Upstream API differences, for reference
| Fork | Upstream `go-zookeeper/zk` |
|---|---|
| `GetW/ChildrenW/ExistsW` return `*Watcher` (with `.EvtCh`) | return
`<-chan Event` |
| `RemoveWatcher`, `ErrWatcherRemoved` | not present (unused by dubbo-go) |
| `TestCluster`, `StartTestCluster`, `WithRetryTimes` importable | test-only
/ not present |
| `Connect`, connOptions, error vars, `Event`/`State`/flags/ACL, `Stat` |
identical |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
