diff --git a/controller/getchangedtargets.go b/controller/getchangedtargets.go index c2c50aa1..9fc18ae7 100644 --- a/controller/getchangedtargets.go +++ b/controller/getchangedtargets.go @@ -414,6 +414,22 @@ func (c *controller) cacheComparedTargets(logger *zap.Logger, request *pb.GetCha // where exactly one revision's blob predates a format flip). func (c *controller) compareFetchedGraphs(ctx context.Context, e *metrics.Emitter, logger *zap.Logger, first, second fetchedGraph, seedAttrs map[string]bool) ([]entity.GetChangedTargetsResponse, error) { if first.tgb != nil && second.tgb != nil { + firstATFH, err := first.tgb.TGB().AllTargetsFileHashes() + if err != nil { + return nil, fmt.Errorf("read AllTargetsFileHashes from first TGB: %w", err) + } + secondATFH, err := second.tgb.TGB().AllTargetsFileHashes() + if err != nil { + return nil, fmt.Errorf("read AllTargetsFileHashes from second TGB: %w", err) + } + if allTargetsFileChanged( + &entity.Metadata{AllTargetsFileHashes: firstATFH}, + &entity.Metadata{AllTargetsFileHashes: secondATFH}, + ) { + logger.Info("compareFetchedGraphs: AllTargetsFiles trigger matched (TGB), reporting all targets as changed") + e.Counter(opGetChangedTargets, "all_targets_triggered").Inc(1) + return c.allTargetsChangedFromTGB(ctx, second.tgb.TGB()) + } return c.compareTargetGraphsTGB(ctx, e, logger, first.tgb.TGB(), second.tgb.TGB(), seedAttrs) } firstChunks, err := first.materializeChunks() @@ -720,6 +736,22 @@ func (c *controller) allTargetsChangedFromGraph(ctx context.Context, targetsByID return c.resultToResponses(targetdiff.Result{ChangedTargets: changed}) } +// allTargetsChangedFromTGB builds a response stream marking every target in +// the TGB reader's graph as changed with distance 0. Used when the +// AllTargetsFiles trigger fires on the TGB comparison path. +func (c *controller) allTargetsChangedFromTGB(ctx context.Context, r *tgb.Reader) ([]entity.GetChangedTargetsResponse, error) { + g, err := r.DecodeGraph() + if err != nil { + return nil, fmt.Errorf("decode TGB graph: %w", err) + } + targetsByID := make(map[int32]*entity.OptimizedTarget, len(g.Targets)) + for i := range g.Targets { + t := &g.Targets[i] + targetsByID[t.ID] = t + } + return c.allTargetsChangedFromGraph(ctx, targetsByID, &g.Metadata) +} + // seedAttributesFor returns the RepositoryConfig.SeedAttributes // allowlist configured for the given remote, or nil when the repository has no // override configured — meaning every attribute is considered a valid signal diff --git a/controller/getchangedtargets_test.go b/controller/getchangedtargets_test.go index 1403e162..2b5f0dc1 100644 --- a/controller/getchangedtargets_test.go +++ b/controller/getchangedtargets_test.go @@ -221,9 +221,10 @@ func TestCompareTargetGraphs_AllTargetsFileTrigger(t *testing.T) { {ID: 1, Hash: "h1", RuleType: 100}, {ID: 2, Hash: "h2", RuleType: 100}, {ID: 3, Hash: "h3", RuleType: 100}, + {ID: 4, Hash: "h4", RuleType: 100, DirectDependencies: []int32{3}}, }}, {Metadata: &entity.Metadata{ - TargetIDMapping: map[int32]string{1: "//app:a", 2: "//app:b", 3: "//app:c"}, + TargetIDMapping: map[int32]string{1: "//app:a", 2: "//app:b", 3: "//app:c", 4: "//lib:util"}, RuleTypeMapping: map[int32]string{100: "go_library"}, AllTargetsFileHashes: map[string]string{".bazelrc": "new-hash"}, }}, @@ -236,7 +237,7 @@ func TestCompareTargetGraphs_AllTargetsFileTrigger(t *testing.T) { for _, resp := range responses { totalChanged += len(resp.ChangedTargets) } - assert.Equal(t, 3, totalChanged, "all targets from second graph should be reported as changed") + assert.Equal(t, 4, totalChanged, "all targets from second graph should be reported as changed") for _, resp := range responses { for _, ct := range resp.ChangedTargets { assert.Equal(t, entity.ChangeTypeChanged, ct.ChangeType) diff --git a/controller/getchangedtargets_tgb_test.go b/controller/getchangedtargets_tgb_test.go index 3485720c..b8df732b 100644 --- a/controller/getchangedtargets_tgb_test.go +++ b/controller/getchangedtargets_tgb_test.go @@ -40,6 +40,8 @@ var ( tgbHash1 = strings.Repeat("aa", 20) tgbHash2Old = strings.Repeat("bb", 20) tgbHash2New = strings.Repeat("cc", 20) + tgbHash3 = strings.Repeat("dd", 20) + tgbHash4 = strings.Repeat("ee", 20) ) // tgbTestGraphChunks builds the two-target graph the gob-era streamChunks @@ -151,6 +153,112 @@ func TestGetChangedTargets_TGBNativePath(t *testing.T) { assert.EqualValues(t, 0, counterValue(scope, "tgb_shadow_error")) } +// tgbTestGraphChunksWithATFH builds a four-target graph with AllTargetsFileHashes +// set in the metadata, for testing the TGB AllTargetsFiles trigger path. +func tgbTestGraphChunksWithATFH(hash2 string, atfh map[string]string) []entity.GetTargetGraphResponse { + return []entity.GetTargetGraphResponse{ + {Targets: []entity.OptimizedTarget{ + {ID: 1, Hash: tgbHash1, RuleType: 100}, + {ID: 2, Hash: hash2, RuleType: 100, DirectDependencies: []int32{1}}, + {ID: 3, Hash: tgbHash3, RuleType: 100}, + {ID: 4, Hash: tgbHash4, RuleType: 100, DirectDependencies: []int32{3}}, + }}, + {Metadata: &entity.Metadata{ + TargetIDMapping: map[int32]string{1: "//app:target1", 2: "//app:target2", 3: "//lib:util", 4: "//lib:core"}, + RuleTypeMapping: map[int32]string{100: "go_library"}, + AllTargetsFileHashes: atfh, + }}, + } +} + +// TestGetChangedTargets_TGBAllTargetsTrigger verifies that the TGB comparison +// path checks AllTargetsFileHashes and, when a configured file differs, +// reports every target in the second graph as changed with distance 0. +func TestGetChangedTargets_TGBAllTargetsTrigger(t *testing.T) { + ctrl := gomock.NewController(t) + stream := tangomock.NewMockTangoServiceGetChangedTargetsYARPCServer(ctrl) + stream.EXPECT().Context().Return(t.Context()) + var sent []*pb.GetChangedTargetsResponse + stream.EXPECT().Send(gomock.Any()).DoAndReturn(func(resp *pb.GetChangedTargetsResponse, _ ...interface{}) error { + sent = append(sent, resp) + return nil + }).AnyTimes() + + st := storage.NewMemoryStorage() + seedTreehash(t, st, "sha1", "treehash1") + seedTreehash(t, st, "sha2", "treehash2") + require.NoError(t, storage.WriteTGBGraph(t.Context(), st, + cachekey.GetTGBGraphByTreeHash("repo:go-code", "treehash1", entity.ComputationStrategyUnset, nil), + tgbTestGraphChunksWithATFH(tgbHash2Old, map[string]string{".bazelrc": "old-hash"}))) + require.NoError(t, storage.WriteTGBGraph(t.Context(), st, + cachekey.GetTGBGraphByTreeHash("repo:go-code", "treehash2", entity.ComputationStrategyUnset, nil), + tgbTestGraphChunksWithATFH(tgbHash2Old, map[string]string{".bazelrc": "new-hash"}))) + + scope := tally.NewTestScope("", nil) + c := NewController(context.Background(), Params{ + Logger: zaptest.NewLogger(t), + Storage: st, + Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), + Scope: scope, + GraphFormat: config.GraphFormatTGB, + }) + + request := changedTargetsRequest() + request.OutputConfig = &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} + require.NoError(t, c.GetChangedTargets(request, stream)) + + changed, _ := changedTargetsSent(t, sent) + require.Len(t, changed, 4, "all targets from second graph should be reported as changed") + for _, ct := range changed { + assert.Equal(t, pb.CHANGE_TYPE_CHANGED, ct.GetChangeType()) + assert.Equal(t, int32(0), ct.GetDistance()) + assert.NotNil(t, ct.GetNewTarget()) + } + assert.EqualValues(t, 1, counterValue(scope, "all_targets_triggered")) + assert.EqualValues(t, 0, counterValue(scope, "tgb_native_compare"), "trigger should skip the normal TGB diff") +} + +// TestGetChangedTargets_TGBAllTargetsNoTrigger verifies that the TGB path +// proceeds with normal comparison when AllTargetsFileHashes match. +func TestGetChangedTargets_TGBAllTargetsNoTrigger(t *testing.T) { + ctrl := gomock.NewController(t) + stream := tangomock.NewMockTangoServiceGetChangedTargetsYARPCServer(ctrl) + stream.EXPECT().Context().Return(t.Context()) + var sent []*pb.GetChangedTargetsResponse + stream.EXPECT().Send(gomock.Any()).DoAndReturn(func(resp *pb.GetChangedTargetsResponse, _ ...interface{}) error { + sent = append(sent, resp) + return nil + }).AnyTimes() + + st := storage.NewMemoryStorage() + seedTreehash(t, st, "sha1", "treehash1") + seedTreehash(t, st, "sha2", "treehash2") + require.NoError(t, storage.WriteTGBGraph(t.Context(), st, + cachekey.GetTGBGraphByTreeHash("repo:go-code", "treehash1", entity.ComputationStrategyUnset, nil), + tgbTestGraphChunksWithATFH(tgbHash2Old, map[string]string{".bazelrc": "same-hash"}))) + require.NoError(t, storage.WriteTGBGraph(t.Context(), st, + cachekey.GetTGBGraphByTreeHash("repo:go-code", "treehash2", entity.ComputationStrategyUnset, nil), + tgbTestGraphChunksWithATFH(tgbHash2New, map[string]string{".bazelrc": "same-hash"}))) + + scope := tally.NewTestScope("", nil) + c := NewController(context.Background(), Params{ + Logger: zaptest.NewLogger(t), + Storage: st, + Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), + Scope: scope, + GraphFormat: config.GraphFormatTGB, + }) + + request := changedTargetsRequest() + request.OutputConfig = &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} + require.NoError(t, c.GetChangedTargets(request, stream)) + + changed, _ := changedTargetsSent(t, sent) + require.Len(t, changed, 1, "only the hash-flipped target should be changed") + assert.EqualValues(t, 0, counterValue(scope, "all_targets_triggered")) + assert.EqualValues(t, 1, counterValue(scope, "tgb_native_compare"), "should use normal TGB diff") +} + // TestGetChangedTargets_TGBMixedFormatFallsBack covers the transitional // window right after a format flip: one revision's graph exists only as a // pre-flip gob stream, the other as a TGB blob. The comparison must fall back diff --git a/internal/tgb/BUILD.bazel b/internal/tgb/BUILD.bazel index 32c324c2..ff05fdf0 100644 --- a/internal/tgb/BUILD.bazel +++ b/internal/tgb/BUILD.bazel @@ -31,5 +31,7 @@ go_test( deps = [ ":tgb", "//entity", + "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", ], ) diff --git a/internal/tgb/encode.go b/internal/tgb/encode.go index 15e051bb..081f29d0 100644 --- a/internal/tgb/encode.go +++ b/internal/tgb/encode.go @@ -736,6 +736,10 @@ func (e *encoder) write(w io.Writer) error { {colBlockStart, codecZstd, blockStartRaw}, } + if len(e.g.Metadata.AllTargetsFileHashes) > 0 { + rawCols = append(rawCols, rawCol{colAllTargetsFileHashes, codecZstd, encodeStringMap(e.g.Metadata.AllTargetsFileHashes)}) + } + // Compress zstd columns (optionally parallel). type compressedCol struct { id uint64 @@ -829,6 +833,27 @@ func (e *encoder) write(w io.Writer) error { return nil } +// encodeStringMap serializes a map[string]string as a length-prefixed sequence +// of (key, value) pairs: count (uvarint), then for each pair the key length +// (uvarint), key bytes, value length (uvarint), value bytes. +func encodeStringMap(m map[string]string) []byte { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + slices.Sort(keys) + + var buf []byte + buf = binary.AppendUvarint(buf, uint64(len(m))) + for _, k := range keys { + buf = binary.AppendUvarint(buf, uint64(len(k))) + buf = append(buf, k...) + buf = binary.AppendUvarint(buf, uint64(len(m[k]))) + buf = append(buf, m[k]...) + } + return buf +} + // TruncateHashes returns a new Graph with each target's Hash field truncated // to n raw bytes and re-hex-encoded. This is necessary for round-trip tests // because TGB only stores the first n bytes of the hash, and Decode can only diff --git a/internal/tgb/format.go b/internal/tgb/format.go index 4950031a..27a72722 100644 --- a/internal/tgb/format.go +++ b/internal/tgb/format.go @@ -104,6 +104,10 @@ const ( // — ensureDepOffsets builds a full offset table and DepsCSR reads in bulk // — so the encoder stopped writing it. Do not reuse the ID. colNodeIndex = uint64(21) + // colAllTargetsFileHashes (22) stores the AllTargetsFileHashes sidecar: + // a length-prefixed sequence of (key, value) string pairs. Old readers + // that don't know this ID skip it; new readers on old blobs get nil. + colAllTargetsFileHashes = uint64(22) ) // colCodec values stored in the directory. @@ -115,27 +119,28 @@ const ( // ─── Column names (for ColumnStats) ────────────────────────────────────────── var colNames = map[uint64]string{ - colPkgDict: "PKG_DICT", - colNameDict: "NAME_DICT", - colNodePkg: "NODE_PKG", - colNodeName: "NODE_NAME", - colHash: "HASH", - colDeg: "DEG", - colDeps: "DEPS", - colRuleType: "RULETYPE", - colRuleTypeDict: "RULETYPE_DICT", - colTagDeg: "TAG_DEG", - colTags: "TAGS", - colTagDict: "TAG_DICT", - colAttrDeg: "ATTR_DEG", - colAttrName: "ATTR_NAME", - colAttrValue: "ATTR_VALUE", - colAttrNameDict: "ATTR_NAME_DICT", - colAttrValueDict: "ATTR_VALUE_DICT", - colFlags: "FLAGS", - colBlockDigest: "BLOCK_DIGEST", - colBlockStart: "BLOCK_START", - colNodeIndex: "NODE_INDEX", + colPkgDict: "PKG_DICT", + colNameDict: "NAME_DICT", + colNodePkg: "NODE_PKG", + colNodeName: "NODE_NAME", + colHash: "HASH", + colDeg: "DEG", + colDeps: "DEPS", + colRuleType: "RULETYPE", + colRuleTypeDict: "RULETYPE_DICT", + colTagDeg: "TAG_DEG", + colTags: "TAGS", + colTagDict: "TAG_DICT", + colAttrDeg: "ATTR_DEG", + colAttrName: "ATTR_NAME", + colAttrValue: "ATTR_VALUE", + colAttrNameDict: "ATTR_NAME_DICT", + colAttrValueDict: "ATTR_VALUE_DICT", + colFlags: "FLAGS", + colBlockDigest: "BLOCK_DIGEST", + colBlockStart: "BLOCK_START", + colNodeIndex: "NODE_INDEX", + colAllTargetsFileHashes: "ALL_TARGETS_FILE_HASHES", } // ─── Header ─────────────────────────────────────────────────────────────────── diff --git a/internal/tgb/reader.go b/internal/tgb/reader.go index 86b9436a..c684cfed 100644 --- a/internal/tgb/reader.go +++ b/internal/tgb/reader.go @@ -179,17 +179,23 @@ func (r *Reader) DecodeGraph() (*Graph, error) { // Build rule-type mapping: dict-index → string (already in ruleTypeMap). // Tag mapping similarly. - g := &Graph{ - Targets: targets, - Metadata: entity.Metadata{ - TargetIDMapping: targetIDMap, - RuleTypeMapping: ruleTypeMap, - TagMapping: tagMap, - AttributeNameMapping: attrNameMap, - AttributeStringValueMapping: attrValMap, - }, - } - return g, nil + meta := entity.Metadata{ + TargetIDMapping: targetIDMap, + RuleTypeMapping: ruleTypeMap, + TagMapping: tagMap, + AttributeNameMapping: attrNameMap, + AttributeStringValueMapping: attrValMap, + } + + atfh, err := r.AllTargetsFileHashes() + if err != nil { + return nil, fmt.Errorf("tgb: decode AllTargetsFileHashes: %w", err) + } + if atfh != nil { + meta.AllTargetsFileHashes = atfh + } + + return &Graph{Targets: targets, Metadata: meta}, nil } // ─── zstd decompression ─────────────────────────────────────────────────────── @@ -1052,6 +1058,49 @@ func (r *Reader) getColumnLocked(id uint64) ([]byte, error) { return out, nil } +// decodeStringMap deserializes the format produced by encodeStringMap: count +// (uvarint), then count (key-length, key, value-length, value) pairs. +func decodeStringMap(data []byte) (map[string]string, error) { + n, sz := binary.Uvarint(data) + if sz <= 0 { + return nil, fmt.Errorf("truncated count") + } + data = data[sz:] + m := make(map[string]string, n) + for i := uint64(0); i < n; i++ { + kLen, sz := binary.Uvarint(data) + if sz <= 0 || uint64(len(data)-sz) < kLen { + return nil, fmt.Errorf("truncated key at entry %d", i) + } + data = data[sz:] + key := string(data[:kLen]) + data = data[kLen:] + + vLen, sz := binary.Uvarint(data) + if sz <= 0 || uint64(len(data)-sz) < vLen { + return nil, fmt.Errorf("truncated value at entry %d", i) + } + data = data[sz:] + m[key] = string(data[:vLen]) + data = data[vLen:] + } + return m, nil +} + +// AllTargetsFileHashes returns the sidecar file-hash map stored in column 22, +// or nil when the column is absent (old blobs or repos without AllTargetsFiles). +// Only the single column is decompressed — no full graph decode. +func (r *Reader) AllTargetsFileHashes() (map[string]string, error) { + if _, ok := r.cols[colAllTargetsFileHashes]; !ok { + return nil, nil + } + data, err := r.getColumn(colAllTargetsFileHashes) + if err != nil { + return nil, err + } + return decodeStringMap(data) +} + func (r *Reader) ensurePkgDict() error { if r.pkgDict != nil { return nil diff --git a/internal/tgb/tgb_test.go b/internal/tgb/tgb_test.go index f85ff4c1..22db69fe 100644 --- a/internal/tgb/tgb_test.go +++ b/internal/tgb/tgb_test.go @@ -9,6 +9,8 @@ import ( "sort" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/uber/tango/entity" "github.com/uber/tango/internal/tgb" ) @@ -169,6 +171,7 @@ func canonicalise(g *tgb.Graph) *tgb.Graph { TagMapping: tagMap, AttributeNameMapping: anMap, AttributeStringValueMapping: avMap, + AllTargetsFileHashes: g.Metadata.AllTargetsFileHashes, }, } } @@ -310,6 +313,58 @@ func TestTinyGraphRoundTrip(t *testing.T) { } } +func TestAllTargetsFileHashesRoundTrip(t *testing.T) { + t.Parallel() + + t.Run("present", func(t *testing.T) { + t.Parallel() + g := buildTinyGraph() + g.Metadata.AllTargetsFileHashes = map[string]string{ + ".bazelrc": "abc123", + "tools/bazel": "def456", + "rules/go/sdk.bzl": "789ghi", + } + + data := mustEncode(t, g, tgb.EncodeOptions{HashBytes: 8, BlockSize: 4}) + got := mustDecode(t, data) + + assert.Equal(t, g.Metadata.AllTargetsFileHashes, got.Metadata.AllTargetsFileHashes) + + r, err := tgb.NewReader(data) + require.NoError(t, err) + atfh, err := r.AllTargetsFileHashes() + require.NoError(t, err) + assert.Equal(t, g.Metadata.AllTargetsFileHashes, atfh) + }) + + t.Run("absent", func(t *testing.T) { + t.Parallel() + g := buildTinyGraph() + + data := mustEncode(t, g, tgb.EncodeOptions{HashBytes: 8, BlockSize: 4}) + got := mustDecode(t, data) + + assert.Nil(t, got.Metadata.AllTargetsFileHashes) + + r, err := tgb.NewReader(data) + require.NoError(t, err) + atfh, err := r.AllTargetsFileHashes() + require.NoError(t, err) + assert.Nil(t, atfh) + }) + + t.Run("empty map not encoded", func(t *testing.T) { + t.Parallel() + g := buildTinyGraph() + g.Metadata.AllTargetsFileHashes = map[string]string{} + + data := mustEncode(t, g, tgb.EncodeOptions{HashBytes: 8, BlockSize: 4}) + got := mustDecode(t, data) + + assert.Nil(t, got.Metadata.AllTargetsFileHashes) + }) +} + // ─── Test: property test over random small graphs ───────────────────────────── func TestRandomGraphRoundTrip(t *testing.T) { diff --git a/internal/tgbdiff/adapt.go b/internal/tgbdiff/adapt.go index ade1105e..bc67e122 100644 --- a/internal/tgbdiff/adapt.go +++ b/internal/tgbdiff/adapt.go @@ -79,6 +79,12 @@ func MergeChunks(chunks []entity.GetTargetGraphResponse) *tgb.Graph { for k, v := range m.AttributeStringValueMapping { g.Metadata.AttributeStringValueMapping[k] = v } + for k, v := range m.AllTargetsFileHashes { + if g.Metadata.AllTargetsFileHashes == nil { + g.Metadata.AllTargetsFileHashes = make(map[string]string, len(m.AllTargetsFileHashes)) + } + g.Metadata.AllTargetsFileHashes[k] = v + } } } return g