Skip to content

Dev zhongli#981

Open
zhongli-james wants to merge 7 commits into
alibaba:developfrom
zhongli-james:dev_zhongli
Open

Dev zhongli#981
zhongli-james wants to merge 7 commits into
alibaba:developfrom
zhongli-james:dev_zhongli

Conversation

@zhongli-james
Copy link
Copy Markdown
Collaborator

some fixes for recent issues: #976 #978 #979

@zhongli-james zhongli-james requested a review from sel-fish May 26, 2026 02:17
@sel-fish
Copy link
Copy Markdown
Collaborator

有两个点需要再看下:

  1. config.chunks 里真实的 MinKey / MaxKey decode 出来应该是 primitive.MinKey{} / primitive.MaxKey{},但现在 getBsonType() 只特殊处理了 int64(math.MinInt64) / int64(math.MaxInt64)。这样跑到首尾 chunk 的时候可能会 panic:getBsonType meet unknown type primitive.MinKey

    现在的 test 里用 int64 sentinel 构造了边界,所以没覆盖到真实 decode 后的类型。建议这里直接支持 primitive.MinKey/MaxKey,测试里也可以用 BSON marshal/unmarshal 出来的值来构造 chunk range。

  2. tests/integration/sharded/README.md 里写了 docker compose up -dsetup_cluster.sh 也依赖 configsvr/shard1/shard2/mongos/dest 这些 service,但 PR 里好像没带 docker-compose.yml。现在这套 integration test 应该还跑不起来。

…oop (alibaba#976)

break inside switch does not exit the enclosing for-range, leaving
disk-stored oplog unreachable. Use labeled break.
…ught up (alibaba#979)

retrieve goroutine hits MemoryApply after restart shortcut and Panicf-s.
Add explicit MemoryApply case that exits the goroutine.
…ibaba#978)

When mongo_s_url is set, full sync routes through mongos and chunk map
is never loaded, so orphan_document=true silently has no effect.

- full.go: emit Warnf when orphan filter is requested but MongoS != nil
- doc_executor.go: skip orphan check on unmarshal failure instead of panic
- unit tests for OrphanFilter range/compound/hashed boundaries
- integration test scaffold (docker-compose + inject + verify 3 cases)
- standalone mode: implement SetDirect(true) for directConnection
- GetColShardType: support int32/int64 shard key values
- getBsonType: keep int64 raw to avoid float64 precision loss above 2^53
- getBsonType: handle primitive.MinKey/MaxKey from real BSON decode
- ComputeHash: add bool/DateTime/Timestamp/nil type support
…s and local paths

Replace hardcoded MongoDB test URIs (containing real internal/public IPs
and plaintext credentials) and a hardcoded personal CA pem path with
env-var driven loading, so the repo never ships environment-specific
secrets and other developers can supply their own values.
@mobing0622
Copy link
Copy Markdown
Collaborator

还有两个点需要再看下:

  1. ShardCollection 现在只有 collection-level 的 ShardTypeFilter() 里只要是 HashedShard 就会对所有 shard key 字段都 ComputeHash。但 compound hashed shard key 里只有 hashed 字段需要 hash,比如 {a: 1, b: "hashed"}a 应该按原值比较,只有 b 应该 hash;反过来 {a: "hashed", b: 1} 也可能因为最后一个字段是 ranged,导致完全不 hash。

    这个 PR 扩了 hashed path 的类型覆盖,建议这里补一个 compound hashed 的 case pin 一下行为;如果当前不准备支持 compound hashed,也可以在加载 shard metadata 时直接报不支持,避免静默误判。

  2. verify_orphan_filter.py 里 Case B 现在只检查了 warning 文案。这个能证明提示打出来了,但不能证明 mongos + orphan_document=true 这条路径真的没有错误过滤数据。建议这里再补一下目标端数据/失败状态的断言,避免后面出现“一边打 warning,一边行为还是不对”的情况。

@zhongli-james
Copy link
Copy Markdown
Collaborator Author

还有两个点需要再看下:

  1. ShardCollection 现在只有 collection-level 的 ShardTypeFilter() 里只要是 HashedShard 就会对所有 shard key 字段都 ComputeHash。但 compound hashed shard key 里只有 hashed 字段需要 hash,比如 {a: 1, b: "hashed"}a 应该按原值比较,只有 b 应该 hash;反过来 {a: "hashed", b: 1} 也可能因为最后一个字段是 ranged,导致完全不 hash。
    这个 PR 扩了 hashed path 的类型覆盖,建议这里补一个 compound hashed 的 case pin 一下行为;如果当前不准备支持 compound hashed,也可以在加载 shard metadata 时直接报不支持,避免静默误判。
  2. verify_orphan_filter.py 里 Case B 现在只检查了 warning 文案。这个能证明提示打出来了,但不能证明 mongos + orphan_document=true 这条路径真的没有错误过滤数据。建议这里再补一下目标端数据/失败状态的断言,避免后面出现“一边打 warning,一边行为还是不对”的情况。

感谢评审 🙏,两点都已修,单独 commit 推上来了。

1. compound hashed shard key

确认是 bug,根因在 GetColShardType 循环里每个字段都覆盖同一个shardType,最终值等同于「最后一个字段的类型」。{a: 1, b: "hashed"}会被记为 HashedShard 然后 a、b 全 hash;{a: "hashed", b: 1} 会被记为 RangedShard 一个都不 hash —— 两种都是静默错判命中文档为 orphan。
按建议的「pin 一下行为」方向修复(per-key 类型,而不是 fail-fast 拒绝),理由:fail-fast 会拒绝一类合法的 4.4+ shard key 用法,而完整支持其实代码量很小。

具体改动:

  • ShardCollection.ShardType stringShardTypes []string,长度与Keys 一致。
  • GetColShardType 返回 per-key 类型切片。
  • Filter() 逐字段判 ShardTypes[keyInd] == HashedShard 决定是否 hash。

回归用例:

  • TestOrphanFilter_CompoundHashed_RangedFirst{a: ranged, b: hashed},断言 a=5 落在 [3, 7) 内 —— 一旦 Filter 错把 a 也 hash,ComputeHash(5)几乎不可能再回到 [3, 7),断言立刻失败。
  • TestOrphanFilter_CompoundHashed_HashedFirst:反向 {a: hashed, b: ranged},钉住 a 被 hash 到 hashA、b 按原值落入 [3, 7)

顺手没做:拒绝「>1 个 hashed 字段」这类组合 —— MongoDB server 本身就不允许创建,运行时不可能出现;如果未来 server 放开,per-key 切片的写法也不会再静默误判。

2. verify_orphan_filter.py Case B

完全同意「只断 warning 文案不够」。已经在 Case B 里补一条与 Case C对齐的行为断言:

  behaved_as_unfiltered = (rc != 0 or cnt < 2)
  case("filter actually disabled (baseline behaviour)",
       behaved_as_unfiltered,
       f"rc={rc} dest_count={cnt} (expect rc!=0 or dest<2)")

即「mongos + orphan_document=true 模式下,dest 必须出现和 Case C(filter 关闭)一样的失败形态」。这样后续如果出现「日志打 warning、行为却被静默改回」的回归,这条断言会先挂。

TestShouldSkipDupKeyOnInsert and TestHandleDupKeyOnInsertStrategy have
been failing since they were added in alibaba#973.

 1. Test fixture mismatch with production
 2. _id_ falls through wildcard rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants