AlexStocks opened a new issue, #3249:
URL: https://github.com/apache/dubbo-go/issues/3249
## Background
dubbo-go has grown organically over 10 years to 826 .go files and ~139K LOC
across 22+ top-level directories. Several structural issues have accumulated
that impact developer productivity, build times, and code quality. This issue
proposes a phased reorganization plan.
Related: #3246 (memory leaks rooted in URL god object), #3247 (data races
rooted in extension global maps), #3248 (general improvements).
---
## Current State Analysis
### Code Distribution
| Directory | Files | LOC | % of Total | Issue |
|-----------|-------|-----|------------|-------|
| `protocol/` | 198 | 46,221 | 33% | God package: 9 protocol
implementations, 10-level nesting |
| `cluster/` | 118 | 13,395 | 10% | Acceptable, good separation |
| `filter/` | 96 | 11,535 | 8% | 3rd-party integrations
(hystrix/sentinel/seata) bundled |
| `registry/` | 67 | 11,523 | 8% | Dual architecture (Registry +
ServiceDiscovery) unclear |
| `config/` | 68 | 8,641 | 6% | 65 files flat in one directory, no
sub-grouping |
| `remoting/` | 46 | 6,721 | 5% | Should be internal to registry, not
top-level |
| `common/extension/` | 22 | ~2,000 | 1% | 22 files with identical pattern,
root cause of #3247 |
| `global/` | 25 | 4,254 | 3% | Overlaps with `config/`, unclear boundary |
| Root `.go` | 8 | 2,727 | 2% | `compat.go` alone is 1,129 LOC |
| `tools/` | 80 | 9,456 | 7% | CLI + codegen + formatter mixed |
### Key Structural Problems
**Problem 1: `protocol/` is a 46K-LOC god package**
9 protocol implementations (dubbo, dubbo3, grpc, triple, rest, jsonrpc,
mock, invocation, result) plus protocolwrapper, all under one directory. The
triple sub-tree nests 10 levels deep, mixing generated and handwritten code:
```
protocol/triple/triple_protocol/internal/gen/proto/connect/collide/v1/*.pb.go
```
Any change to the shared `protocol/base/` interface forces rebuilding all 9
implementations. Code review scope is unpredictable.
**Problem 2: `config/` has 65 files flat in one directory**
```
config/
├── application_config.go
├── consumer_config.go
├── provider_config.go
├── service_config.go (602 LOC)
├── reference_config.go (544 LOC)
├── root_config.go (401 LOC)
├── config_loader.go
├── ... 58 more files ...
```
No sub-grouping by concern. New contributors can't find anything without
grep.
**Problem 3: `common/extension/` is 22 copy-pasted files**
Every file follows the exact same pattern — a global `map[string]func()`,
plus `Set*`/`Get*`/`Unregister*` functions with zero lock protection. This is
the root cause of 20+ data races (#3247). The pattern was duplicated 22 times
instead of being abstracted.
**Problem 4: `config/` vs `global/` boundary is unclear**
`global/` (25 files, 4,254 LOC) holds configuration struct definitions.
`config/` (68 files, 8,641 LOC) holds loading/initialization logic. But some
config structs live in `config/`, some in `global/`. Callers don't know which
package to import.
**Problem 5: `remoting/` is misplaced**
`remoting/` contains network clients for zookeeper, nacos, etcdv3, polaris,
getty. But these are implementation details of `registry/` and
`config_center/`, not a separate concern. Having `remoting/` at the top level
makes the dependency direction confusing: `registry/nacos/` imports
`remoting/nacos/`, which is essentially one package split across two
directories.
**Problem 6: Root `compat.go` is 1,129 LOC**
A backwards-compatibility shim layer sitting in the project root alongside
the core entry point `dubbo.go`. This mixes concerns and inflates the root
package.
**Problem 7: 67 direct dependencies**
Every protocol, registry backend, filter integration, and tool is bundled in
one `go.mod`. Users who only need triple+nacos still pull in zookeeper, etcd,
polaris, hystrix, sentinel, seata, and 60+ other dependencies.
---
## Reorganization Plan
### Phase 1: Zero-risk cleanups (no API changes, no import path changes)
These changes only move internal code and add new abstractions. No external
API breaks.
#### Step 1.1: Introduce `extension.Registry[T]` to replace 22 copy-pasted
files
**Before** (22 files, each ~80 LOC):
```
common/extension/
├── filter.go # var filters = make(map[string]...)
├── protocol.go # var protocols = make(map[string]...)
├── registry.go # var registries = make(map[string]...)
├── cluster.go # var clusters = make(map[string]...)
├── loadbalance.go # ...same pattern...
├── configurator.go
├── config_center.go
├── config_center_factory.go
├── config.go
├── metadata_report_factory.go
├── proxy_factory.go
├── tps_limit.go
├── auth.go
├── rest_client.go
├── rest_server.go
├── config_reader.go
├── config_post_processor.go
├── router_factory.go
├── registry_directory.go
├── service_discovery.go
├── service_instance_selector_factory.go
├── logger.go
└── otel_trace.go
```
**After** (3 files):
```
common/extension/
├── registry.go # Registry[T] generic container (NEW)
├── extensions.go # All extension variable declarations using
Registry[T]
└── graceful_shutdown.go # Shutdown callbacks (different pattern, keep
separate)
```
**Implementation**:
```go
// common/extension/registry.go
package extension
import "sync"
// Registry is a thread-safe, generic extension registry.
// It replaces the 22 hand-rolled global maps that previously had no lock
protection.
type Registry[T any] struct {
mu sync.RWMutex
items map[string]T
name string // for error messages
}
func NewRegistry[T any](name string) *Registry[T] {
return &Registry[T]{
items: make(map[string]T),
name: name,
}
}
func (r *Registry[T]) Register(name string, v T) {
r.mu.Lock()
defer r.mu.Unlock()
r.items[name] = v
}
func (r *Registry[T]) Get(name string) (T, bool) {
r.mu.RLock()
defer r.mu.RUnlock()
v, ok := r.items[name]
return v, ok
}
func (r *Registry[T]) MustGet(name string) T {
r.mu.RLock()
defer r.mu.RUnlock()
v, ok := r.items[name]
if !ok {
panic(r.name + " for " + name + " is not registered")
}
return v
}
func (r *Registry[T]) Unregister(name string) {
r.mu.Lock()
defer r.mu.Unlock()
delete(r.items, name)
}
// Snapshot returns a copy of all registered items. Safe to iterate.
func (r *Registry[T]) Snapshot() map[string]T {
r.mu.RLock()
defer r.mu.RUnlock()
m := make(map[string]T, len(r.items))
for k, v := range r.items {
m[k] = v
}
return m
}
// Names returns a copy of all registered names.
func (r *Registry[T]) Names() []string {
r.mu.RLock()
defer r.mu.RUnlock()
names := make([]string, 0, len(r.items))
for k := range r.items {
names = append(names, k)
}
return names
}
```
```go
// common/extension/extensions.go
package extension
// All extension registries, replacing 22 individual global maps.
var (
Filters = NewRegistry[func() filter.Filter]("filter")
Protocols = NewRegistry[func() base.Protocol]("protocol")
Registries = NewRegistry[func(*common.URL) (registry.Registry,
error)]("registry")
Clusters = NewRegistry[func() cluster.Cluster]("cluster")
LoadBalances = NewRegistry[func()
loadbalance.LoadBalance]("loadbalance")
// ... all others ...
)
// Backwards-compatible wrappers (deprecate over time):
func SetFilter(name string, v func() filter.Filter) {
Filters.Register(name, v) }
func GetFilter(name string) (filter.Filter, bool) { f, ok :=
Filters.Get(name); if !ok { return nil, false }; return f(), true }
// ... etc for all existing Set*/Get* functions ...
```
This single change fixes all 20+ data races in `common/extension/` (#3247
Category 1) while maintaining full backwards compatibility through wrapper
functions.
#### Step 1.2: Move `compat.go` to `compat/` sub-package
```bash
mkdir -p compat/
mv compat.go compat/compat.go
# Update package declaration and add re-export aliases in root if needed
```
Root directory after this change:
```
dubbo.go (350 LOC) — Instance API
loader.go (515 LOC) — Config loading
options.go (542 LOC) — InstanceOptions
compat/compat.go (1,129 LOC) — Backwards compatibility (moved)
```
#### Step 1.3: Centralize generated code under `gen/`
**Before**:
```
protocol/triple/triple_protocol/internal/gen/proto/connect/collide/v1/*.pb.go
(10 levels)
protocol/triple/triple_protocol/internal/gen/proto/connect/import/v1/*.pb.go
protocol/triple/triple_protocol/internal/gen/proto/connect/ping/v1/*.pb.go
```
**After**:
```
protocol/triple/
├── gen/ # All generated code (max 3 levels)
│ ├── collide_v1/
│ ├── import_v1/
│ └── ping_v1/
├── triple_protocol/ # Handwritten code only
│ ├── client.go
│ ├── server.go
│ └── ...
├── client.go
└── server.go
```
Add `gen/` to `.gitattributes` so GitHub collapses generated files in PRs:
```
protocol/triple/gen/** linguist-generated=true
```
---
### Phase 2: Config reorganization (internal refactor, deprecate old paths)
#### Step 2.1: Merge `global/` into `config/model/`
`global/` is purely data structures (no logic). It should be a sub-package
of `config/`:
**Before**:
```
global/
├── application_config.go # struct ApplicationConfig
├── consumer_config.go # struct ConsumerConfig
├── provider_config.go # struct ProviderConfig
├── registry_config.go # struct RegistryConfig
├── service_config.go # struct ServiceConfig
├── shutdown_config.go # struct ShutdownConfig
├── tls_config.go # struct TLSConfig
├── tracing_config.go # struct TracingConfig
├── method_config.go # struct MethodConfig
└── ... 16 more ...
```
**After**:
```
config/
├── model/ # Pure struct definitions (was global/)
│ ├── application.go
│ ├── consumer.go
│ ├── provider.go
│ ├── registry.go
│ └── ...
├── loader/ # Config loading & initialization
│ ├── loader.go # was config_loader.go
│ └── root.go # was root_config.go
└── ...
```
Migration path: keep `global/` as a thin re-export wrapper for one major
version:
```go
// global/application_config.go (deprecated)
package global
import "dubbo.apache.org/dubbo-go/v3/config/model"
type ApplicationConfig = model.ApplicationConfig // type alias, zero cost
```
#### Step 2.2: Group `config/` files by concern
**Before** (65 files flat):
```
config/
├── application_config.go
├── consumer_config.go
├── provider_config.go
├── service_config.go (602 LOC)
├── reference_config.go (544 LOC)
├── root_config.go (401 LOC)
├── config_loader.go
├── graceful_shutdown.go
├── metadata_report_config.go
├── ... 56 more files flat ...
```
**After** (grouped by domain):
```
config/
├── model/ # Pure structs (from global/)
├── loader/ # Root config + loading + env
│ ├── loader.go
│ ├── root.go
│ └── env.go
├── service/ # Provider-side config
│ ├── service_config.go
│ ├── method_config.go
│ └── export.go
├── reference/ # Consumer-side config
│ ├── reference_config.go
│ └── method_config.go
├── registry/ # Registry config
│ └── registry_config.go
├── metadata/ # Metadata config
│ └── metadata_config.go
├── validation/ # Config validation (extracted from Init()
methods)
│ └── validator.go
└── service.go # GetProviderService/GetConsumerService (keep at
root)
```
---
### Phase 3: Merge `remoting/` into its consumers
`remoting/` is not a standalone concern — it's network client code that only
exists to serve `registry/` and `config_center/`.
**Before**:
```
remoting/
├── getty/ → used by protocol/dubbo (keep as-is, it IS a standalone
network layer)
├── nacos/ → used ONLY by registry/nacos/ and config_center/nacos/
├── zookeeper/ → used ONLY by registry/zookeeper/ and
config_center/zookeeper/
├── etcdv3/ → used ONLY by registry/etcdv3/ and config_center/etcdv3/
└── polaris/ → used ONLY by registry/polaris/
```
**After**:
```
remoting/
└── getty/ # Keep: genuine standalone network layer
registry/
├── nacos/
│ ├── registry.go
│ └── client.go # Was remoting/nacos/ — internal network client
├── zookeeper/
│ ├── registry.go
│ └── client.go # Was remoting/zookeeper/
├── etcdv3/
│ ├── registry.go
│ └── client.go # Was remoting/etcdv3/
└── polaris/
├── registry.go
└── client.go # Was remoting/polaris/
config_center/
├── nacos/
│ └── client.go # Shared with registry/nacos/ via internal package
├── zookeeper/
│ └── client.go
└── etcdv3/
└── client.go
```
If `registry/nacos/` and `config_center/nacos/` share the same client code,
extract to a shared internal package:
```
internal/nacos/client.go # shared nacos client
registry/nacos/ # imports internal/nacos
config_center/nacos/ # imports internal/nacos
```
---
### Phase 4: Protocol modularization (major version, long-term)
Split each protocol into an independent Go module so users only pull in what
they need.
**Before** (single go.mod, 67 direct dependencies):
```
go.mod ← everything bundled
```
**After** (multi-module):
```
go.mod # Core framework (common, cluster,
filter, registry interfaces)
protocol/triple/go.mod # Triple protocol module
protocol/grpc/go.mod # gRPC protocol module
protocol/dubbo/go.mod # Dubbo protocol module (legacy)
protocol/rest/go.mod # REST protocol module
protocol/jsonrpc/go.mod # JSON-RPC protocol module
```
User imports only what they need:
```go
import (
"dubbo.apache.org/dubbo-go/v3"
_ "dubbo.apache.org/dubbo-go/v3/protocol/triple" // only triple
_ "dubbo.apache.org/dubbo-go/v3/registry/nacos" // only nacos
)
```
Benefits:
- Users who only use triple+nacos don't pull in zookeeper, etcd, polaris,
grpc dependencies
- Each protocol can have its own release cycle
- Build times decrease significantly
- `go.sum` shrinks dramatically
#### Step 4.2: Extract 3rd-party filter integrations
These filters depend on external SDKs and should not be in the core
repository:
```
# Move to separate repos:
filter/hystrix/ → github.com/apache/dubbo-go-filter-hystrix
filter/sentinel/ → github.com/apache/dubbo-go-filter-sentinel
filter/seata/ → github.com/apache/dubbo-go-filter-seata
# Keep in core (no external SDK dependency):
filter/active/
filter/accesslog/
filter/generic/
filter/token/
filter/echo/
filter/tps/
filter/exec_limit/
filter/graceful_shutdown/
filter/metrics/
filter/otel/
```
---
### Phase 5: Fix `dubbo.go` root package lock bug + cleanup
#### Step 5.1: Fix wrong lock (one-line, do immediately)
```go
// dubbo.go:300-306
func SetProviderService(svc common.RPCService) {
proLock.Lock() // FIX: was conLock.Lock()
defer proLock.Unlock() // FIX: was conLock.Unlock()
providerServices[common.GetReference(svc)] = &server.ServiceDefinition{
Handler: svc,
}
}
```
#### Step 5.2: Add missing lock to GetConsumerConnection
```go
// dubbo.go:308-311
func GetConsumerConnection(interfaceName string) (*client.Connection, error)
{
conLock.RLock()
defer conLock.RUnlock()
return consumerServices[interfaceName].GetConnection()
}
```
---
## Summary & Ordering
| Phase | Effort | Risk | Impact | Prerequisite |
|-------|--------|------|--------|-------------|
| 1.1 `extension.Registry[T]` | 1 week | Zero (backwards compatible
wrappers) | Fixes 20+ races | None |
| 1.2 Move `compat.go` | 1 hour | Zero | Root package clarity | None |
| 1.3 Centralize generated code | 1 day | Zero | Readability | None |
| 5.1 Fix wrong lock | 1 minute | Zero | Fixes confirmed crash | None |
| 5.2 Add missing lock | 1 minute | Zero | Fixes race | None |
| 2.1 Merge `global/` → `config/model/` | 1 week | Low (type aliases for
compat) | Clarity | None |
| 2.2 Group `config/` files | 2 weeks | Medium (internal import paths
change) | Navigability | 2.1 |
| 3 Merge `remoting/` | 2 weeks | Medium (internal import paths change) |
Clean boundaries | None |
| 4.1 Protocol multi-module | 1 month | High (module split) | Dependency
reduction | 1.1, 2.1 |
| 4.2 Extract 3rd-party filters | 2 weeks | Medium (new repos) | Dependency
reduction | 4.1 |
Phases 1 and 5 can start immediately with zero risk. Phases 2-3 can proceed
in parallel. Phase 4 requires a major version bump.
--
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]
