Copilot commented on code in PR #1151:
URL:
https://github.com/apache/skywalking-banyandb/pull/1151#discussion_r3341915174
##########
banyand/trace/write_data_segmentref_test.go:
##########
@@ -229,13 +226,13 @@ func
TestSyncChunkCallback_CreatePartHandler_StoresSegment(t *testing.T) {
segTime := time.Date(2026, 4, 17, 0, 0, 0, 0, time.Local)
- // Drive the segment into the idle-closed state (refCount=0) so that the
- // probe at the end of this test reliably exercises the `initialize`
- // branch of incRef if and only if Close actually DecRef'd.
+ // Drop the segment to refCount 0. Under the dormant-refcount model it
stays
+ // open (loaded), so subsequent CreateSegmentIfNotExist calls
re-acquire it
+ // without reloading shards.
warmup, err := db.CreateSegmentIfNotExist(segTime)
require.NoError(t, err)
warmup.DecRef()
- warmup.DecRef() // refCount -> 0
+ warmup.DecRef() // refCount = 0 (dormant: open, no active reference)
Review Comment:
Same as above: CreateSegmentIfNotExist returns a segment with a single
active reference, so calling DecRef twice breaks the intended contract and
reduces the test's ability to catch ref-count regressions. A single DecRef
should leave the segment dormant (refCount==0).
##########
banyand/trace/write_data_segmentref_test.go:
##########
@@ -74,12 +74,13 @@ func TestSyncReceiver_SegmentRefOwnership(t *testing.T) {
segTime := time.Date(2026, 4, 17, 0, 0, 0, 0, time.UTC)
- // Drive the segment into the cold-0 "idle-closed" state: segment object
- // still exists in sc.lst but refCount=0 and shards have been released.
+ // Drop the segment to refCount 0. Under the dormant-refcount model it
stays
+ // open (loaded), so re-acquiring it does NOT reload shards: the open
count
+ // stays flat across the sibling sync sessions below.
seg, err := db.CreateSegmentIfNotExist(segTime)
require.NoError(t, err)
- seg.DecRef() // normal caller release -> refCount = 1
- seg.DecRef() // simulate idle close -> refCount = 0
+ seg.DecRef()
+ seg.DecRef() // refCount = 0 (dormant: open, no active reference)
Review Comment:
The segment returned by CreateSegmentIfNotExist carries exactly one active
reference; calling DecRef twice here violates the documented caller contract
(one DecRef per incRef) and can mask reference accounting bugs. A single DecRef
should drop refCount to 0 under the dormant model.
--
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]
