Change the data storage path structure for property model #955

mrproliu · 2026-01-22T03:30:00Z

If this pull request closes/resolves/fixes an existing issue, replace the issue number. Fixes apache/skywalking#.
Update the CHANGES log.

Copilot

Pull request overview

Updates the property model’s on-disk layout to be group-aware, so shards are stored under per-group directories, and adjusts repair/snapshot/gossip logic and tests accordingly.

Changes:

Store property shards under <property-dir>/<group>/shard-<id>/... and load/query/delete across group-scoped shard sets.
Update repair tree building, snapshotting, and repair gossip flows to operate with group-scoped shards/snapshots.
Refresh generated API reference docs and add a breaking-change note to CHANGES.md.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
docs/api-reference.md	Updates generated API docs for TopNList.Item fields.
banyand/property/test_helper.go	Updates test helper to use new `loadShard(group, id)` signature.
banyand/property/shard_test.go	Adjusts shard tests for group-aware shard loading.
banyand/property/shard.go	Writes shards under a group directory and tags metrics with `group`.
banyand/property/repair_test.go	Updates repair tests for group-aware shard loading and tree reader signature.
banyand/property/repair_gossip_test.go	Updates gossip tests to snapshot/load shards per group.
banyand/property/repair_gossip.go	Loads shards by group and uses the updated tree reader API.
banyand/property/repair.go	Adjusts repair tree/state paths and rebuild logic for the new layout.
banyand/property/listener.go	Takes snapshots into group-scoped directories.
banyand/property/db.go	Introduces group→shards mapping and routes operations by group.
CHANGES.md	Documents the breaking storage layout change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-22T03:36:28Z

banyand/property/db.go

+		(*groupsMap)[group] = gs
+		db.groups.Store(groupsMap)


getOrCreateGroupShards mutates the existing map[string]*groupShards in-place ((*groupsMap)[group] = gs) and then re-stores the same map pointer. Callers like delete/query/collect/close iterate over *groupsMap without holding db.mu, so this can trigger concurrent map iteration and map write panics. Use copy-on-write (allocate a new map, copy entries, add the new group, then Store the new pointer) or protect all reads with the same lock used for writes.

Suggested change

(*groupsMap)[group] = gs

db.groups.Store(groupsMap)

// Use copy-on-write: create a new map, copy existing entries, then add the new group.

oldMap := *groupsMap

newMap := make(map[string]*groupShards, len(oldMap)+1)

for k, v := range oldMap {

newMap[k] = v

}

newMap[group] = gs

db.groups.Store(&newMap)

Use sync.Map to fix it.

Copilot · 2026-01-22T03:36:28Z

banyand/property/db.go

+	gs.mu.Lock()
+	sLst := gs.shards.Load()
 	if sLst == nil {
 		sLst = &[]*shard{}
 	}
-	*sLst = append(*sLst, sd)
-	db.sLst.Store(sLst)
+	newList := append(*sLst, sd)
+	gs.shards.Store(&newList)


newList := append(*sLst, sd) can reuse the backing array of *sLst and mutate it in-place. Since readers access gs.shards.Load() without taking gs.mu, this can cause data races / inconsistent reads. Build the new slice with a guaranteed new backing array (e.g., copy to a fresh slice before appending) before storing it.

banyand/property/repair.go

CHANGES.md

Co-authored-by: Copilot <[email protected]>

banyand/property/db.go

hanahmily · 2026-01-22T03:44:09Z

banyand/property/db.go

+	gs.mu.Lock()
+	sLst := gs.shards.Load()
 	if sLst == nil {
 		sLst = &[]*shard{}
 	}
-	*sLst = append(*sLst, sd)
-	db.sLst.Store(sLst)
+	newList := append(*sLst, sd)
+	gs.shards.Store(&newList)


hanahmily · 2026-01-22T03:44:55Z

banyand/property/db.go

+		(*groupsMap)[group] = gs
+		db.groups.Store(groupsMap)


Use sync.Map to fix it.

codecov-commenter · 2026-01-22T03:58:36Z

Codecov Report

❌ Patch coverage is 50.00000% with 103 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.19%. Comparing base (3530dd9) to head (9ddd2fc).
⚠️ Report is 120 commits behind head on main.

Files with missing lines	Patch %	Lines
banyand/property/db.go	53.40%	35 Missing and 6 partials ⚠️
banyand/property/listener.go	0.00%	30 Missing ⚠️
banyand/property/repair.go	64.78%	15 Missing and 10 partials ⚠️
banyand/property/repair_gossip.go	50.00%	3 Missing and 3 partials ⚠️
banyand/property/test_helper.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #955      +/-   ##
==========================================
+ Coverage   45.97%   47.19%   +1.21%     
==========================================
  Files         328      383      +55     
  Lines       55505    59611    +4106     
==========================================
+ Hits        25520    28131    +2611     
- Misses      27909    28849     +940     
- Partials     2076     2631     +555

Flag	Coverage Δ
banyand	`49.65% <50.00%> (?)`
bydbctl	`81.91% <ø> (?)`
fodc	`78.59% <ø> (?)`
integration-distributed	`80.00% <ø> (?)`
pkg	`29.32% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

banyand/property/listener.go:272

This always returns a Snapshot{Name: sn,...} even if no shards were actually snapshotted (e.g., db.groups is empty or all gs.shards are nil), which means no snapshot directory gets created. Previously the code returned a nil payload when there were no shards. Consider returning nil/an error Snapshot in the “nothing snapshotted” case or creating the snapshot directory structure up-front.

	return bus.NewMessage(bus.MessageID(time.Now().UnixNano()), &databasev1.Snapshot{
		Name:    sn,
		Catalog: commonv1.Catalog_CATALOG_PROPERTY,
	})

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

banyand/property/listener.go

CHANGES.md

Co-authored-by: Copilot <[email protected]>

Change the data storage path structure for property model

104d72a

mrproliu added this to the 0.10.0 milestone Jan 22, 2026

mrproliu requested review from Copilot and hanahmily January 22, 2026 03:30

mrproliu added the enhancement New feature or request label Jan 22, 2026

Copilot started reviewing on behalf of mrproliu January 22, 2026 03:30 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

hanahmily and others added 2 commits January 22, 2026 11:37

Merge branch 'main' into grouped-property

982a654

Apply suggestions from code review

f5a0f87

Co-authored-by: Copilot <[email protected]>

hanahmily requested changes Jan 22, 2026

View reviewed changes

mrproliu added 2 commits January 22, 2026 12:13

fix issues

ccf23a1

fix UT

96af006

hanahmily requested a review from Copilot January 22, 2026 05:21

Copilot started reviewing on behalf of hanahmily January 22, 2026 05:22 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

banyand/property/listener.go Show resolved Hide resolved

CHANGES.md Outdated Show resolved Hide resolved

mrproliu and others added 3 commits January 22, 2026 13:34

Update CHANGES.md

63bb7c3

Co-authored-by: Copilot <[email protected]>

Update banyand/property/listener.go

9c9ae8d

Co-authored-by: Copilot <[email protected]>

fix lint

9ddd2fc

hanahmily approved these changes Jan 22, 2026

View reviewed changes

hanahmily merged commit 5e585bc into apache:main Jan 22, 2026
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change the data storage path structure for property model #955

Change the data storage path structure for property model #955

Uh oh!

mrproliu commented Jan 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

hanahmily Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

hanahmily Jan 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hanahmily Jan 22, 2026

Uh oh!

hanahmily Jan 22, 2026

Uh oh!

codecov-commenter commented Jan 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-		(*groupsMap)[group] = gs
-		db.groups.Store(groupsMap)
+		// Use copy-on-write: create a new map, copy existing entries, then add the new group.
+		oldMap := *groupsMap
+		newMap := make(map[string]*groupShards, len(oldMap)+1)
+		for k, v := range oldMap {
+			newMap[k] = v
+		}
+		newMap[group] = gs
+		db.groups.Store(&newMap)

Change the data storage path structure for property model #955

Change the data storage path structure for property model #955

Uh oh!

Conversation

mrproliu commented Jan 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

hanahmily Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

hanahmily Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hanahmily Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

hanahmily Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Jan 22, 2026 •

edited

Loading