Review Response

blockstore/splitstore/splitstore_compact.go

@@ -204,7 +204,7 @@ func (s *SplitStore) markLiveRefs(cids []cid.Cid) {
 
 	count := new(int32)
 	visitor := newConcurrentVisitor()
-	walkObject := func(c cid.Cid) (int, error) {
+	walkObject := func(c cid.Cid) (int64, error) {
 		return s.walkObjectIncomplete(c, visitor,
 			func(c cid.Cid) error {
 				if isUnitaryObject(c) {
@@ -426,7 +426,7 @@ func (s *SplitStore) protectTxnRefs(markSet MarkSet) error {
 
 // transactionally protect a reference by walking the object and marking.
 // concurrent markings are short circuited by checking the markset.
-func (s *SplitStore) doTxnProtect(root cid.Cid, markSet MarkSet) (int, error) {
+func (s *SplitStore) doTxnProtect(root cid.Cid, markSet MarkSet) (int64, error) {
 	if err := s.checkClosing(); err != nil {
 		return 0, err
 	}
@@ -1082,8 +1082,8 @@ func (s *SplitStore) walkChain(ts *types.TipSet, inclState, inclMsgs abi.ChainEp
 	return nil
 }
 
-func (s *SplitStore) walkObject(c cid.Cid, visitor ObjectVisitor, f func(cid.Cid) error) (int, error) {
+func (s *SplitStore) walkObject(c cid.Cid, visitor ObjectVisitor, f func(cid.Cid) error) (int64, error) {
-	var sz int
+	var sz int64
 	visit, err := visitor.Visit(c)
 	if err != nil {
 		return 0, xerrors.Errorf("error visiting object: %w", err)
@@ -1112,7 +1112,7 @@ func (s *SplitStore) walkObject(c cid.Cid, visitor ObjectVisitor, f func(cid.Cid
 
 	var links []cid.Cid
 	err = s.view(c, func(data []byte) error {
-		sz += len(data)
+		sz += int64(len(data))
 		return cbg.ScanForLinks(bytes.NewReader(data), func(c cid.Cid) {
 			links = append(links, c)
 		})
@@ -1134,8 +1134,8 @@ func (s *SplitStore) walkObject(c cid.Cid, visitor ObjectVisitor, f func(cid.Cid
 }
 
 // like walkObject, but the object may be potentially incomplete (references missing)
-func (s *SplitStore) walkObjectIncomplete(c cid.Cid, visitor ObjectVisitor, f, missing func(cid.Cid) error) (int, error) {
+func (s *SplitStore) walkObjectIncomplete(c cid.Cid, visitor ObjectVisitor, f, missing func(cid.Cid) error) (int64, error) {
-	sz := 0
+	sz := int64(0)
 	visit, err := visitor.Visit(c)
 	if err != nil {
 		return 0, xerrors.Errorf("error visiting object: %w", err)
@@ -1181,7 +1181,7 @@ func (s *SplitStore) walkObjectIncomplete(c cid.Cid, visitor ObjectVisitor, f, m
 
 	var links []cid.Cid
 	err = s.view(c, func(data []byte) error {
-		sz += len(data)
+		sz += int64(len(data))
 		return cbg.ScanForLinks(bytes.NewReader(data), func(c cid.Cid) {
 			links = append(links, c)
 		})

blockstore/splitstore/splitstore_gc.go

@@ -28,18 +28,21 @@ func (s *SplitStore) gcHotAfterCompaction() {
 	// 	a) If we should not do full GC => online GC
 	//  b) If we should do full GC and can => moving GC
 	//  c) If we should do full GC and can't => aggressive online GC
-	var hotSize int64
+	getSize := func() int64 {
-	var err error
 		sizer, ok := s.hot.(bstore.BlockstoreSize)
 		if ok {
-		hotSize, err = sizer.Size()
+			size, err := sizer.Size()
 			if err != nil {
 				log.Warnf("error getting hotstore size: %s, estimating empty hot store for targeting", err)
-			hotSize = 0
+				return 0
 			}
+			return size
 		} else {
-		hotSize = 0
+			log.Errorf("Could not measure hotstore size, assuming it is 0 bytes, which it is not")
+			return 0
 		}
+	}
+	hotSize := getSize()
 
 	copySizeApprox := s.szKeys + s.szMarkedLiveRefs + s.szProtectedTxns + s.szWalk
 	shouldTarget := s.cfg.HotstoreMaxSpaceTarget > 0 && hotSize+copySizeApprox > int64(s.cfg.HotstoreMaxSpaceTarget)-targetThreshold
@@ -63,6 +66,7 @@ func (s *SplitStore) gcHotAfterCompaction() {
 	if err := s.gcBlockstore(s.hot, opts); err != nil {
 		log.Warnf("error garbage collecting hostore: %s", err)
 	}
+	log.Infof("measured hot store size after GC: %d", getSize())
 }
 
 func (s *SplitStore) gcBlockstore(b bstore.Blockstore, opts []bstore.BlockstoreGCOption) error {

node/config/doc_gen.go

@@ -1295,7 +1295,10 @@ will run moving GC if disk utilization gets within a threshold (150 GB) of the t
 Splitstore GC will NOT run moving GC if the total size of the move would get
 within 50 GB of the target, and instead will run a more aggressive online GC.
 If both HotStoreFullGCFrequency and HotStoreMaxSpaceTarget are set then splitstore
-GC will trigger moving GC if either configuration condition is met.`,
+GC will trigger moving GC if either configuration condition is met.
+A reasonable minimum is 2x fully GCed hotstore size + 50 G buffer.
+At this minimum size moving GC happens every time, any smaller and moving GC won't
+be able to run. In spring 2023 this minimum is ~550 GB.`,
 		},
 	},
 	"StorageMiner": []DocField{

node/config/types.go

@@ -607,6 +607,9 @@ type Splitstore struct {
 	// within 50 GB of the target, and instead will run a more aggressive online GC.
 	// If both HotStoreFullGCFrequency and HotStoreMaxSpaceTarget are set then splitstore
 	// GC will trigger moving GC if either configuration condition is met.
+	// A reasonable minimum is 2x fully GCed hotstore size + 50 G buffer.
+	// At this minimum size moving GC happens every time, any smaller and moving GC won't
+	// be able to run. In spring 2023 this minimum is ~550 GB.
 	HotStoreMaxSpaceTarget uint64
 }