Revert "[SharovBot] fix(history): correct .vi index page offset for V1 paged snapshot files" (#19440)

Reverts #19411

seems merged without team agreement

Author: AskAlexSharov

db/seg/seg_paged_rw.go

@@ -27,7 +27,7 @@ import (
 
 var be = binary.BigEndian
 
-func GetFromPage(key, compressedPage []byte, compressionBuf []byte, compressionEnabled bool) (v []byte, ok bool, compressionBufOut []byte) {
+func GetFromPage(key, compressedPage []byte, compressionBuf []byte, compressionEnabled bool) (v []byte, compressionBufOut []byte) {
 	var err error
 	var page []byte
 	compressionBuf, page, err = compress.DecodeZstdIfNeed(compressionBuf[:0], compressedPage, compressionEnabled)
@@ -37,7 +37,7 @@ func GetFromPage(key, compressedPage []byte, compressionBuf []byte, compressionE
 
 	cnt := int(page[0])
 	if cnt == 0 {
-		return nil, false, compressionBuf
+		return nil, compressionBuf
 	}
 	meta, data := page[1:1+cnt*4*2], page[1+cnt*4*2:]
 	kLens, vLens := meta[:cnt*4], meta[cnt*4:]
@@ -53,14 +53,14 @@ func GetFromPage(key, compressedPage []byte, compressionBuf []byte, compressionE
 		kLen, vLen := be.Uint32(kLens[i:]), be.Uint32(vLens[i:])
 		foundKey := keys[kOffset : kOffset+kLen]
 		if bytes.Equal(key, foundKey) {
-			return vals[vOffset : vOffset+vLen], true, compressionBuf
+			return vals[vOffset : vOffset+vLen], compressionBuf
 		} else {
 			_ = data
 		}
 		kOffset += kLen
 		vOffset += vLen
 	}
-	return nil, false, compressionBuf
+	return nil, compressionBuf
 }
 
 type Page struct {

db/seg/seg_paged_rw_test.go

@@ -132,8 +132,7 @@ func TestPage(t *testing.T) {
 	for i := 0; i < sampling+1; i++ {
 		iter++
 		expectK, expectV := fmt.Sprintf("k %d", i), fmt.Sprintf("v %d", i)
-		v, ok, _ := GetFromPage([]byte(expectK), pages[pageNum], nil, false)
-		require.True(ok)
+		v, _ := GetFromPage([]byte(expectK), pages[pageNum], nil, false)
 		require.Equal(expectV, string(v), i)
 		require.True(p1.HasNext())
 		k, v := p1.Next()
@@ -172,83 +171,6 @@ func TestPagedReaderWithCompression(t *testing.T) {
 	require.Equal(len(loremStrings), i, "should have read all entries")
 }
 
-func TestGetFromPageKeyNotFound(t *testing.T) {
-	require := require.New(t)
-
-	// Create a page with 4 key-value pairs
-	pageSize := 4
-	buf := &multyBytesWriter{pageSize: pageSize}
-	w := NewPagedWriter(buf, false)
-	for i := 0; i < pageSize; i++ {
-		k, v := fmt.Sprintf("k %d", i), fmt.Sprintf("v %d", i)
-		require.NoError(w.Add([]byte(k), []byte(v)))
-	}
-	require.NoError(w.Flush())
-	pages := buf.Bytes()
-	require.Len(pages, 1)
-
-	// Existing keys should be found
-	for i := 0; i < pageSize; i++ {
-		k := fmt.Sprintf("k %d", i)
-		v, ok, _ := GetFromPage([]byte(k), pages[0], nil, false)
-		require.True(ok, "key %q should be found", k)
-		require.Equal(fmt.Sprintf("v %d", i), string(v))
-	}
-
-	// Non-existing key should NOT be found
-	v, ok, _ := GetFromPage([]byte("nonexistent"), pages[0], nil, false)
-	require.False(ok, "nonexistent key should not be found")
-	require.Nil(v)
-
-	// Key with similar prefix should NOT be found
-	v, ok, _ = GetFromPage([]byte("k 99"), pages[0], nil, false)
-	require.False(ok, "key 'k 99' should not be found")
-	require.Nil(v)
-}
-
-// TestGetFromPageNonFullPages verifies that when pages have varying numbers
-// of entries (e.g., a last page with fewer entries than the page size),
-// GetFromPage still finds keys correctly. This tests the scenario that
-// caused the .vi index page offset regression.
-func TestGetFromPageNonFullPages(t *testing.T) {
-	require := require.New(t)
-
-	// Create pages with pageSize=4 but 5 entries total
-	// This produces: page0 with 4 entries, page1 with 1 entry
-	pageSize := 4
-	totalEntries := pageSize + 1
-	buf := &multyBytesWriter{pageSize: pageSize}
-	w := NewPagedWriter(buf, false)
-	for i := 0; i < totalEntries; i++ {
-		k, v := fmt.Sprintf("k %d", i), fmt.Sprintf("v %d", i)
-		require.NoError(w.Add([]byte(k), []byte(v)))
-	}
-	require.NoError(w.Flush())
-	pages := buf.Bytes()
-	require.Len(pages, 2, "should have 2 pages: one full and one partial")
-
-	// All 4 keys on page 0 should be found on page 0
-	for i := 0; i < pageSize; i++ {
-		k := fmt.Sprintf("k %d", i)
-		v, ok, _ := GetFromPage([]byte(k), pages[0], nil, false)
-		require.True(ok, "key %q should be found on page 0", k)
-		require.Equal(fmt.Sprintf("v %d", i), string(v))
-	}
-
-	// The 5th key should NOT be on page 0
-	_, ok, _ := GetFromPage([]byte("k 4"), pages[0], nil, false)
-	require.False(ok, "key 'k 4' should NOT be on page 0")
-
-	// The 5th key should be on page 1
-	v, ok, _ := GetFromPage([]byte("k 4"), pages[1], nil, false)
-	require.True(ok, "key 'k 4' should be found on page 1")
-	require.Equal("v 4", string(v))
-
-	// Keys from page 0 should NOT be on page 1
-	_, ok, _ = GetFromPage([]byte("k 0"), pages[1], nil, false)
-	require.False(ok, "key 'k 0' should NOT be on page 1")
-}
-
 func BenchmarkName(b *testing.B) {
 	buf := &multyBytesWriter{pageSize: 16}
 	w := NewPagedWriter(buf, false)

db/state/history.go

@@ -234,104 +234,6 @@ func (h *History) buildVi(ctx context.Context, item *FilesItem, ps *background.P
 }
 
 func (h *History) buildVI(ctx context.Context, historyIdxPath string, hist, efHist *seg.Decompressor, efBaseTxNum uint64, ps *background.ProgressSet) error {
-	// file not the config is the source of truth for the .v file compression state
-	compressedPageValuesCount := hist.CompressedPageValuesCount()
-	if hist.CompressionFormatVersion() == seg.FileCompressionFormatV0 {
-		compressedPageValuesCount = h.HistoryValuesOnCompressedPage
-	}
-
-	if compressedPageValuesCount > 1 {
-		return h.buildVIFromPages(ctx, historyIdxPath, hist, ps)
-	}
-	return h.buildVIFromEF(ctx, historyIdxPath, hist, efHist, efBaseTxNum, ps)
-}
-
-// buildVIFromPages builds the .vi index by reading keys directly from the .v file's pages.
-// This is used for V1 files with page-level compression where each page contains multiple
-// key-value pairs. Reading keys from the pages ensures the index correctly maps each key
-// to its containing page, regardless of whether the .ef file has the same entry ordering.
-func (h *History) buildVIFromPages(ctx context.Context, historyIdxPath string, hist *seg.Decompressor, ps *background.ProgressSet) error {
-	defer hist.MadvSequential().DisableReadAhead()
-
-	histReader := h.dataReader(hist)
-
-	// Count total entries by scanning all pages
-	cnt := 0
-	page := &seg.Page{}
-	histReader.Reset(0)
-	for histReader.HasNext() { //TODO: use vFile.Count()
-		pageData, _ := histReader.Next(nil)
-		page.Reset(pageData, true)
-		for page.HasNext() {
-			page.Next()
-			cnt++
-		}
-		select {
-		case <-ctx.Done():
-			return ctx.Err()
-		default:
-		}
-	}
-
-	_, fName := filepath.Split(historyIdxPath)
-	p := ps.AddNew(fName, uint64(cnt))
-	defer ps.Delete(p)
-	rs, err := recsplit.NewRecSplit(recsplit.RecSplitArgs{
-		KeyCount:   cnt,
-		Enums:      false,
-		BucketSize: recsplit.DefaultBucketSize,
-		LeafSize:   recsplit.DefaultLeafSize,
-		TmpDir:     h.dirs.Tmp,
-		IndexFile:  historyIdxPath,
-		Salt:       h.salt.Load(),
-		NoFsync:    h.noFsync,
-	}, h.logger)
-	if err != nil {
-		return fmt.Errorf("create recsplit: %w", err)
-	}
-	defer rs.Close()
-	rs.LogLvl(log.LvlTrace)
-
-	for {
-		histReader.Reset(0)
-		var valOffset uint64
-
-		for histReader.HasNext() {
-			pageData, nextOffset := histReader.Next(nil)
-			page.Reset(pageData, true)
-			for page.HasNext() {
-				k, _ := page.Next()
-				if err = rs.AddKey(k, valOffset); err != nil {
-					return err
-				}
-			}
-			p.Processed.Add(1)
-			valOffset = nextOffset
-
-			select {
-			case <-ctx.Done():
-				return ctx.Err()
-			default:
-			}
-		}
-
-		if err = rs.Build(ctx); err != nil {
-			if rs.Collision() {
-				log.Info("Building recsplit. Collision happened. It's ok. Restarting...")
-				rs.ResetNextSalt()
-			} else {
-				return fmt.Errorf("build idx: %w", err)
-			}
-		} else {
-			break
-		}
-	}
-	return nil
-}
-
-// buildVIFromEF builds the .vi index using the .ef inverted index file to enumerate keys.
-// This is used for V0 files or files without page-level compression.
-func (h *History) buildVIFromEF(ctx context.Context, historyIdxPath string, hist, efHist *seg.Decompressor, efBaseTxNum uint64, ps *background.ProgressSet) error {
 	var histKey []byte
 	var valOffset uint64
 
@@ -342,8 +244,8 @@ func (h *History) buildVIFromEF(ctx context.Context, historyIdxPath string, hist
 
 	var keyBuf, valBuf []byte
 	cnt := uint64(0)
-	for iiReader.HasNext() { //TODO: use vFile.Count()
-		keyBuf, _ = iiReader.Next(keyBuf[:0])
+	for iiReader.HasNext() {
+		keyBuf, _ = iiReader.Next(keyBuf[:0]) // skip key
 		valBuf, _ = iiReader.Next(valBuf[:0])
 		cnt += multiencseq.Count(efBaseTxNum, valBuf)
 		select {
@@ -377,6 +279,7 @@ func (h *History) buildVIFromEF(ctx context.Context, historyIdxPath string, hist
 	seq := &multiencseq.SequenceReader{}
 	it := &multiencseq.SequenceIterator{}
 
+	i := 0
 	for {
 		histReader.Reset(0)
 		iiReader.Reset(0)
@@ -387,6 +290,8 @@ func (h *History) buildVIFromEF(ctx context.Context, historyIdxPath string, hist
 			valBuf, _ = iiReader.Next(valBuf[:0])
 			p.Processed.Add(1)
 
+			// fmt.Printf("ef key %x\n", keyBuf)
+
 			seq.Reset(efBaseTxNum, valBuf)
 			it.Reset(seq, 0)
 			for it.HasNext() {
@@ -398,7 +303,22 @@ func (h *History) buildVIFromEF(ctx context.Context, historyIdxPath string, hist
 				if err = rs.AddKey(histKey, valOffset); err != nil {
 					return err
 				}
-				valOffset, _ = histReader.Skip()
+
+				// file not the config is the source of truth for the .v file compression state
+				compressedPageValuesCount := hist.CompressedPageValuesCount()
+
+				if hist.CompressionFormatVersion() == seg.FileCompressionFormatV0 {
+					compressedPageValuesCount = h.HistoryValuesOnCompressedPage
+				}
+
+				if compressedPageValuesCount == 0 {
+					valOffset, _ = histReader.Skip()
+				} else {
+					i++
+					if i%compressedPageValuesCount == 0 {
+						valOffset, _ = histReader.Skip()
+					}
+				}
 			}
 
 			select {
@@ -1335,6 +1255,7 @@ func (ht *HistoryRoTx) historySeekInFiles(key []byte, txNum uint64) ([]byte, boo
 	}
 	g := ht.statelessGetter(historyItem.i)
 	g.Reset(offset)
+	//fmt.Printf("[dbg] hist.seek: offset=%d\n", offset)
 	v, _ := g.Next(nil)
 	if traceGetAsOf == ht.h.FilenameBase {
 		fmt.Printf("DomainGetAsOf(%s, %x, %d) -> %s, histTxNum=%d, isNil(v)=%t\n", ht.h.FilenameBase, key, txNum, g.FileName(), histTxNum, v == nil)
@@ -1347,13 +1268,7 @@ func (ht *HistoryRoTx) historySeekInFiles(key []byte, txNum uint64) ([]byte, boo
 	}
 
 	if compressedPageValuesCount > 1 {
-		v, ok, ht.snappyReadBuffer = seg.GetFromPage(historyKey, v, ht.snappyReadBuffer, true)
-		if !ok {
-			// Key not found in page. The inverted index (.ef) contains a txNum
-			// entry that has no corresponding entry in the history values (.v)
-			// file. Fall back to DB/latest lookup to get the correct value.
-			return nil, false, nil
-		}
+		v, ht.snappyReadBuffer = seg.GetFromPage(historyKey, v, ht.snappyReadBuffer, true)
 	}
 	return v, true, nil
 }
@@ -1655,10 +1570,7 @@ func (ht *HistoryRoTx) HistoryDump(fromTxNum, toTxNum int, keyToDump *[]byte, du
 
 				if compressedPageValuesCount > 0 {
 					histKeyBuf = historyKey(txNum, key, histKeyBuf)
-					val, ok, _ = seg.GetFromPage(histKeyBuf, val, nil, true)
-					if !ok {
-						return fmt.Errorf("HistoryDump: not found key [%x] on compressed page: %s", key, viFile.Fullpath())
-					}
+					val, _ = seg.GetFromPage(histKeyBuf, val, nil, true)
 				}
 
 				dumpTo(key, txNum, val)