mrproliu commented on code in PR #1046:
URL: 
https://github.com/apache/skywalking-banyandb/pull/1046#discussion_r3035655085


##########
banyand/internal/storage/tsdb_test.go:
##########
@@ -308,6 +309,55 @@ func TestTakeFileSnapshot(t *testing.T) {
 
                require.NoError(t, tsdb.Close())
        })
+
+       t.Run("Take snapshot skips shard with no current snapshot", func(t 
*testing.T) {
+               dir, defFn := test.Space(require.New(t))
+               defer defFn()
+
+               snapshotDir := filepath.Join(dir, "snapshot")
+
+               opts := TSDBOpts[*SnapshotMockTSTable, any]{
+                       Location:        dir,
+                       SegmentInterval: IntervalRule{Unit: DAY, Num: 1},
+                       TTL:             IntervalRule{Unit: DAY, Num: 7},
+                       ShardNum:        1,
+                       TSTableCreator:  SnapshotMockTSTableCreator,
+               }
+
+               ctx := context.Background()
+               mc := timestamp.NewMockClock()
+               ts, err := time.ParseInLocation("2006-01-02 15:04:05", 
"2024-05-01 00:00:00", time.Local)
+               require.NoError(t, err)
+               mc.Set(ts)
+               ctx = timestamp.SetClock(ctx, mc)
+
+               serviceCache := NewServiceCache()
+               tsdb, err := OpenTSDB(ctx, opts, serviceCache, group)
+               require.NoError(t, err)
+               defer tsdb.Close()
+
+               normalSeg, err := tsdb.CreateSegmentIfNotExist(ts)
+               require.NoError(t, err)
+               normalSeg.DecRef()
+
+               epochSeg, err := tsdb.CreateSegmentIfNotExist(time.Unix(0, 0))
+               require.NoError(t, err)
+               epochSeg.DecRef()
+
+               created, snapshotErr := tsdb.TakeFileSnapshot(snapshotDir)
+               require.NoError(t, snapshotErr,
+                       "snapshot should not fail due to empty shard in epoch 
segment")
+               require.True(t, created)
+
+               normalSegDir := filepath.Join(snapshotDir,
+                       fmt.Sprintf("seg-%s", ts.Format("20060102")))
+               require.DirExists(t, normalSegDir,
+                       "normal segment should be present in snapshot")
+
+               epochSegDir := filepath.Join(snapshotDir, "seg-19700101")
+               require.DirExists(t, epochSegDir,
+                       "epoch segment directory should still be created")

Review Comment:
   done



##########
banyand/internal/storage/tsdb.go:
##########
@@ -309,6 +309,11 @@ func (d *database[T, O]) TakeFileSnapshot(dst string) 
(bool, error) {
                        shardPath := filepath.Join(segPath, shardDir)
                        d.lfs.MkdirIfNotExist(shardPath, DirPerm)
                        if _, err := shard.table.TakeFileSnapshot(shardPath); 
err != nil {
+                               if errors.Is(err, ErrNoCurrentSnapshot) {
+                                       log.Warn().Str("shard", 
shardDir).Str("segment", segDir).

Review Comment:
   done



##########
CHANGES.md:
##########
@@ -19,6 +19,7 @@ Release Notes.
 - MCP: Add validation for properties and harden the mcp server.
 - Fix property schema client connection not stable after data node restarted.
 - Fix flaky on-disk integration tests caused by Ginkgo v2 random container 
shuffling closing gRPC connections prematurely.
+- Fix take snapshot error when no data in the segment.

Review Comment:
   done



-- 
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]

Reply via email to