Copilot commented on code in PR #1136:
URL:
https://github.com/apache/skywalking-banyandb/pull/1136#discussion_r3256416978
##########
banyand/trace/write_data_segmentref_test.go:
##########
@@ -213,6 +213,12 @@ func openTestTSDBForRefTest(t *testing.T, tmpPath string,
openShardCount *atomic
//
// This complements TestSyncReceiver_SegmentRefOwnership, which builds
// syncPartContext manually. Together they cover every line of the fix.
+//
+// segTime must use time.Local: the callback path runs time.Unix(0,ns)
+// -> Standard -> CreateSegmentIfNotExist in Local tz. A UTC warmup
+// segment would land on a different calendar day under +0800 (etc),
+// causing the callback to try to create a new segment whose on-disk
+// path collides with the warmup's path.
Review Comment:
The timezone rationale here is a bit inaccurate: under positive offsets like
+0800, a UTC midnight and a Local midnight are usually on the same calendar
day. The real issue is that using UTC vs Local can represent different instants
that may still format to the same segment suffix (YYYYMMDD/HH) and therefore
collide on disk. Please reword to describe the instant-vs-suffix collision
rather than “different calendar day under +0800”.
##########
banyand/stream/write_data_segmentref_test.go:
##########
@@ -114,6 +114,11 @@ func TestSyncReceiver_SegmentRefOwnership(t *testing.T) {
// syncCallback.CreatePartHandler through a minimal schemaRepo shim and
// asserts that segment ownership transfers to partCtx and that Close
// actually DecRefs. See trace package for rationale.
+// In each StoresSegment test, segTime must use time.Local because the
+// callback path runs time.Unix(0,ns) -> Standard -> CreateSegmentIfNotExist
+// in Local tz. A UTC warmup segment would land on a different calendar
+// day under +0800 (etc), causing the callback to try to create a new
+// segment whose path collides with the warmup's path.
Review Comment:
This comment’s timezone example seems backwards: +0800 typically keeps a UTC
midnight on the same calendar day. The underlying problem is mixing UTC and
Local instants that can still format to the same segment suffix, causing
directory-name collisions. Please rephrase to focus on the instant/suffix
mismatch rather than “different calendar day under +0800”.
--
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]
