2 Commits
Author SHA1 Message Date
Chris LuandGitHub a2ff9cca27 master: let VolumeList ask for the volumes it wants (#10674)
* master: let VolumeList ask for the volumes it wants

The request carried nothing, so every caller was answered with the whole
cluster. A dashboard opening one volume's page, or a capacity probe adding up
one bucket, was served all 800k of them and threw away the rest -- and the
master built every one of those messages first.

The topology, its disks and their counters are still reported in full: a caller
reading free space or replica placement needs the cluster whichever volumes it
asked about. Only what is listed under a disk is selected, ec shards included.

An empty collection and a zero volume id take everything, the way volume.list
already reads its own -collectionPattern and -volumeId, so a caller that
forgets to narrow is answered too much rather than answered wrongly. That
leaves the default collection unnameable, since it is the one the empty string
names, so it gets a field of its own.

An older client sends none of it and is answered exactly as before.

* admin: ask the master for the volume the page is showing

A volume's detail page was pulling every volume in the cluster to find one and
its replicas, and discarding the rest.

* admin: ask the master for the ec volume the page is showing

Same as the volume detail page: one volume's shards were found by pulling every
ec shard in the cluster.

* s3: ask the master for the bucket's own collection

The SOSAPI capacity probe summed one collection's volumes out of a listing of
every volume in the cluster. Cluster capacity still comes out the same: it is
read from the disk counters, which a filtered listing reports in full.

* topology: read the disk usage counters atomically

They are written with atomic.AddInt64 from heartbeats but were read plainly by
the two listings and by FreeSpace, and the map they sit in was iterated without
the lock its neighbour takes. Under -race a listing concurrent with a heartbeat
trips on both.
2026-08-09 21:59:42 -07:00
Chris LuandGitHub f1f720f5da fix(master): register EC shards per physical disk on full heartbeat sync (#9212) (#9219)
* refactor(types): add DiskId type for physical-disk identifiers

Names the uint32 physical-disk index that volume servers carry in
VolumeEcShardInformationMessage / VolumeInformationMessage, so EC shard
tracking that needs to distinguish disks within a DataNode can use a
dedicated type instead of an untyped uint32. No behaviour change.

* fix(master): register EC shards per physical disk on full heartbeat sync (#9212)

When a volume's EC shards are spread across multiple physical disks on the
same volume server (common after ec.balance / ec.rebuild on multi-disk
nodes), the volume server emits one VolumeEcShardInformationMessage per
(disk, volume) in its heartbeat. The master's DataNode.UpdateEcShards was
building a `map[VolumeId]*EcVolumeInfo` with last-write-wins, and
doUpdateEcShards then overwrote `disk.ecShards[vid]` once per message, so
all but the final disk's shards were silently dropped. Only the
topology-global ecShardMap (built via RegisterEcShards in a per-message
loop) stayed correct, which hid the problem from `topo.LookupEcShards`
but broke everything that reads the DataNode/Disk view — volume.list,
admin UI, ec.rebuild dry-run ("only 6 shards, skipping"), and
`DiskInfo.EcShardInfos` which the shell's ec.balance / ec.rebuild
planners group by `eci.DiskId`.

Change the shape of `Disk.ecShards` from
    map[VolumeId]*EcVolumeInfo
to
    map[VolumeId]map[types.DiskId]*EcVolumeInfo

so every physical disk keeps its own entry. UpdateEcShards aggregates
incoming messages by (vid, diskId) rather than vid alone; Add/Delete/
HasVolumesById and HasEcShards consult the nested map; doUpdateEcShards
rewrites the nested structure from the aggregated map. Per-physical-disk
attribution survives through DataNode.ToDataNodeInfo ->
DiskInfo.EcShardInfos, matching the wire format the volume server
produces and what downstream admin tooling expects.

Delta sync (AddOrUpdateEcShard / DeleteEcShard) already merged via
ShardsInfo.Add, so this only affects the full-sync path that runs on
heartbeat reconnect.

Adds data_node_ec_multi_disk_test.go with two regression tests that fail
on pre-fix master:
- TestEcShardsAcrossMultipleDisksOnSameNode: volume 15 spread over 3
  disks (matches the bug report's volume-2 row); asserts every shard
  visible via LookupEcShards, DataNode.GetEcShards, and ToDataNodeInfo's
  per-disk EcShardInfos entries.
- TestEcShardsAfterRestartHeartbeat: minimal 2-disk full sync case.

* fix(topology): tighten locking around EC shard map access

Addresses review comments on #9219:

* DataNode.UpdateEcShards now holds dn.Lock for the full read-diff-write
  cycle, matching UpdateVolumes' model, so concurrent heartbeats can no
  longer interleave their getOrCreateDisk / UpAdjustDiskUsageDelta
  updates with each other. Introduces a private getEcShardsLocked helper
  for reads under the held lock; renames doUpdateEcShards to
  doUpdateEcShardsLocked for the same reason.

* DataNode.HasEcShards now takes each disk's ecShardsLock while reading
  disk.ecShards, closing a pre-existing map race with concurrent
  Add/Delete/Update writers.

* doUpdateEcShardsLocked takes each disk's ecShardsLock around the
  reset-and-rewrite so readers (GetEcShards, HasEcShards) see a
  consistent map state rather than a partially-rebuilt one.

* Disk.GetEcShards' slice-capacity hint now accounts for the nested
  per-physical-disk entries (sum of inner lengths) instead of
  underestimating by the unique-volume count.
2026-04-24 14:01:09 -07:00