diff --git a/TESTING.md b/TESTING.md index e0118f47b9b..384f84f9e44 100644 --- a/TESTING.md +++ b/TESTING.md @@ -87,7 +87,7 @@ programmatic control over local Dgraph clusters. Most newer integration2 and upg ## Module Structure -The main module is `github.com/hypermodeinc/dgraph` +The main module is `github.com/dgraph-io/dgraph` The codebase is organized into several key packages: diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index d89a8c0cc0a..6442965a3c2 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -283,6 +283,16 @@ they form a Raft group and provide synchronous replication. "with structured fields including trace ID for correlation with distributed traces. "+ "Disabled by default (0). Note: enabling this logs query text which may contain "+ "sensitive data; do not enable in deployments with strict data privacy requirements."). + Flag("mutations-pipeline-threshold", + "Per-mutation edge-count threshold for routing through the per-predicate "+ + "mutation pipeline, which parallelizes work across distinct predicates "+ + "in a single mutation. 0 disables the pipeline entirely (all mutations "+ + "take the legacy path). 1 always uses the pipeline. A value N>1 uses "+ + "the pipeline only when len(edges) >= N, leaving small interactive "+ + "mutations on the legacy path — the pipeline pays per-predicate "+ + "goroutine spin-up cost, so tiny mutations are slightly slower on it; "+ + "bulk multi-predicate mutations are faster (crossover ~100 edges in "+ + "benchmarks here)."). String()) RegisterFlags(flag) @@ -810,6 +820,7 @@ func run() { x.Config.NormalizeCompatibilityMode = featureFlagsConf.GetString("normalize-compatibility-mode") enableDetailedMetrics := featureFlagsConf.GetBool("enable-detailed-metrics") x.WorkerConfig.SlowQueryLogThreshold = featureFlagsConf.GetDuration("log-slow-query-threshold") + x.WorkerConfig.MutationsPipelineThreshold = int(featureFlagsConf.GetInt64("mutations-pipeline-threshold")) x.PrintVersion() glog.Infof("x.Config: %+v", x.Config) diff --git a/graphql/admin/admin.go b/graphql/admin/admin.go index 5b484b03958..b6411dd8dac 100644 --- a/graphql/admin/admin.go +++ b/graphql/admin/admin.go @@ -655,6 +655,10 @@ func newAdminResolver( return } + if len(pl.Postings) == 2 && string(pl.Postings[0].Value) == "_STAR_ALL" { + pl.Postings = pl.Postings[1:] + } + // There should be only one posting. if len(pl.Postings) != 1 { glog.Errorf("Only one posting is expected in the graphql schema posting list but got %d", diff --git a/posting/index.go b/posting/index.go index 75f8f8a5f41..5d91492dad8 100644 --- a/posting/index.go +++ b/posting/index.go @@ -14,15 +14,19 @@ import ( "fmt" "math" "os" + "sort" "strings" + "sync" "sync/atomic" "time" "unsafe" + "github.com/dgryski/go-farm" "github.com/golang/glog" "github.com/pkg/errors" ostats "go.opencensus.io/stats" "go.opentelemetry.io/otel/trace" + "golang.org/x/sync/errgroup" "google.golang.org/protobuf/proto" "github.com/dgraph-io/badger/v4" @@ -63,23 +67,1169 @@ func indexTokens(ctx context.Context, info *indexMutationInfo) ([]string, error) } sv, err := types.Convert(info.val, schemaType) if err != nil { - return nil, err + return nil, errors.Wrap(err, "Cannot convert value to scalar type") } var tokens []string for _, it := range info.tokenizers { toks, err := tok.BuildTokens(sv.Value, tok.GetTokenizerForLang(it, lang)) if err != nil { - return tokens, err + return tokens, errors.Wrapf(err, "Cannot build tokens for attribute %s", attr) } tokens = append(tokens, toks...) } return tokens, nil } -// addIndexMutations adds mutation(s) for a single term, to maintain the index, -// but only for the given tokenizers. -// TODO - See if we need to pass op as argument as t should already have Op. +// MutationPipeline applies a batch of DirectedEdges by grouping them per predicate +// and processing each predicate in its own goroutine. The key performance advantage +// over the legacy runMutation loop is that each predicate goroutine accumulates all +// edges for the predicate into an in-memory map first, then issues one Badger write +// per distinct uid — O(distinct_uids) reads instead of the legacy O(edges) reads. +// Index, reverse, and count deltas are computed in one pass over the accumulated map. +type MutationPipeline struct { + txn *Txn +} + +func NewMutationPipeline(txn *Txn) *MutationPipeline { + return &MutationPipeline{txn: txn} +} + +// PredicatePipeline is the per-predicate worker. The buffered edges channel decouples +// the dispatcher (Process) from the processor (ProcessPredicate): the dispatcher can +// fan out all predicates before any goroutine has finished, so predicate goroutines run +// in parallel without the dispatcher blocking. Capacity 1000 is enough for most Raft +// proposal batches; the dispatcher never blocks because it closes each channel after +// the full scan. +type PredicatePipeline struct { + attr string + edges chan *pb.DirectedEdge + wg *sync.WaitGroup + errCh chan error +} + +func (pp *PredicatePipeline) close() { + pp.wg.Done() +} + +func (mp *MutationPipeline) ProcessVectorIndex(ctx context.Context, pipeline *PredicatePipeline, info predicateInfo) error { + // Vector inserts must run serially within a txn. HNSW Insert does + // multi-key read-modify-write across the entry-pointer key, level edge + // lists, and neighbor back-edges; per-list locks make individual key + // updates atomic but the cross-key sequences are not. Concurrent + // inserters on the same txn cache lose updates and leave nodes + // unreachable from the entry point — visible as similar_to(k=N) returning + // fewer than N hits even though all N vectors are in the data list. The + // legacy applyMutations path arrives at the same result indirectly: + // x.DivideAndRule rounds num<256 down to numGo=1, so any vector mutation + // short of a few hundred edges runs single-threaded on main. Match that + // here. (TestVectorTwoTxnWithoutCommit reliably reproduced the corruption + // on Linux CI when this used 10 workers.) + for edge := range pipeline.edges { + uid := edge.Entity + key := x.DataKey(pipeline.attr, uid) + pl, err := mp.txn.Get(key) + if err != nil { + return err + } + if err := pl.AddMutationWithIndex(ctx, edge, mp.txn); err != nil { + return err + } + } + return nil +} + +func (mp *MutationPipeline) InsertTokenizerIndexes(ctx context.Context, pipeline *PredicatePipeline, postings *map[uint64]*pb.PostingList, info predicateInfo) error { + tokenizers := schema.State().Tokenizer(ctx, pipeline.attr) + if len(tokenizers) == 0 { + return nil + } + + indexesGenInMutation := types.NewLockedShardedMap[string, *MutableLayer]() + wg := &sync.WaitGroup{} + + syncMap := sync.Map{} + + chanFn := func(uids chan uint64, estimatedSize int) { + defer wg.Done() + indexGenInThread := make(map[string]*pb.PostingList, estimatedSize) + tokenizers := schema.State().Tokenizer(ctx, pipeline.attr) + + factorySpecs, err := schema.State().FactoryCreateSpec(ctx, pipeline.attr) + if err != nil { + pipeline.errCh <- err + return + } + + indexEdge := &pb.DirectedEdge{ + Attr: pipeline.attr, + } + + for uid := range uids { + postingList := (*postings)[uid] + // Sort Dels before Sets: when DEL and SET land in the same index + // bucket (e.g. case-insensitive tokenizer where the old and new + // values share a token), insertPosting's last-write-wins means SET + // must come last to keep the uid in the bucket. + newList := &pb.PostingList{ + Postings: make([]*pb.Posting, len(postingList.Postings)), + } + copy(newList.Postings, postingList.Postings) + sort.SliceStable(newList.Postings, func(i, j int) bool { + return newList.Postings[i].Op == Del && newList.Postings[j].Op != Del + }) + for _, posting := range newList.Postings { + info := &indexMutationInfo{ + tokenizers: tokenizers, + factorySpecs: factorySpecs, + op: pb.DirectedEdge_SET, + val: types.Val{ + Tid: types.TypeID(posting.ValType), + Value: posting.Value, + }, + } + + info.edge = &pb.DirectedEdge{ + Attr: pipeline.attr, + Op: pb.DirectedEdge_SET, + Lang: string(posting.LangTag), + Value: posting.Value, + } + + key := fmt.Sprintf("%s,%s", posting.LangTag, posting.Value) + tokens, loaded := syncMap.Load(key) + + if !loaded { + tokens, err = indexTokens(ctx, info) + if err != nil { + x.Panic(err) + } + syncMap.Store(key, tokens) + } + + indexEdge.Op = GetPostingOp(posting.Op) + indexEdge.ValueId = uid + mpost := makePostingFromEdge(mp.txn.StartTs, indexEdge) + + for _, token := range tokens.([]string) { + key := x.IndexKey(pipeline.attr, token) + val, ok := indexGenInThread[string(key)] + if !ok { + val = &pb.PostingList{} + } + val.Postings = append(val.Postings, mpost) + indexGenInThread[string(key)] = val + } + } + } + + for key, value := range indexGenInThread { + indexesGenInMutation.Update(key, func(val *MutableLayer, ok bool) *MutableLayer { + if !ok { + val = newMutableLayer() + val.currentEntries = &pb.PostingList{} + } + for _, posting := range value.Postings { + val.insertPosting(posting, false) + } + return val + }) + } + } + + numGo := 10 + wg.Add(numGo) + chMap := make(map[int]chan uint64) + + for i := 0; i < numGo; i++ { + uidCh := make(chan uint64, numGo) + chMap[i] = uidCh + go chanFn(uidCh, len(*postings)/numGo) + } + + for uid := range *postings { + // uid is uint64; converting directly to int can produce a negative + // value for uid >= 2^63, which would index outside chMap and resolve + // to a nil channel (deadlocks the dispatcher). Hash unsigned, then + // cast. + chMap[int(uid%uint64(numGo))] <- uid + } + + for i := 0; i < numGo; i++ { + close(chMap[i]) + } + + wg.Wait() + + mp.txn.cache.Lock() + defer mp.txn.cache.Unlock() + + indexGenInTxn := mp.txn.cache.deltas.GetIndexMapForPredicate(pipeline.attr) + if indexGenInTxn == nil { + indexGenInTxn = types.NewLockedShardedMap[string, *pb.PostingList]() + mp.txn.cache.deltas.indexMap[pipeline.attr] = indexGenInTxn + } + + updateFn := func(key string, value *MutableLayer) { + indexGenInTxn.Update(key, func(val *pb.PostingList, ok bool) *pb.PostingList { + if !ok { + val = &pb.PostingList{} + } + val.Postings = append(val.Postings, value.currentEntries.Postings...) + return val + }) + } + + if info.hasUpsert { + err := indexesGenInMutation.Iterate(func(key string, value *MutableLayer) error { + updateFn(key, value) + mp.txn.addConflictKey(farm.Fingerprint64([]byte(key))) + return nil + }) + if err != nil { + return err + } + } else { + err := indexesGenInMutation.Iterate(func(key string, value *MutableLayer) error { + updateFn(key, value) + mp.txn.addConflictKeyWithUid([]byte(key), value.currentEntries, info.hasUpsert, info.noConflict) + return nil + }) + if err != nil { + return err + } + } + + return nil +} + +type predicateInfo struct { + isList bool + index bool + reverse bool + count bool + noConflict bool + hasUpsert bool + isUid bool + + isSingleEdge bool +} + +// ProcessList handles list predicates (multi-valued) and language predicates. +// It first drains all edges into a uid→MutableLayer map (pure in-memory, zero Badger +// reads in this phase), then runs reverse/index/count passes over the accumulated map. +// This is the key difference from the legacy runMutation per-edge loop: a mutation +// touching 500 entities for the same list predicate costs 500 Badger writes here vs +// 500 Badger reads + 500 writes in the legacy path. +func (mp *MutationPipeline) ProcessList(ctx context.Context, pipeline *PredicatePipeline, info predicateInfo) error { + su, schemaExists := schema.State().Get(ctx, pipeline.attr) + + mutations := make(map[uint64]*MutableLayer, 1000) + // rawIndexPostings preserves every batch posting in arrival order, bypassing + // insertPosting's collapse-on-Uid. For scalar @lang predicates mpost.Uid is + // fingerprint(Lang), so SET-new and DEL-old on the same lang collide in + // mutations and one of the two index emissions would be lost. + var rawIndexPostings map[uint64]*pb.PostingList + if info.index { + rawIndexPostings = make(map[uint64]*pb.PostingList, 1000) + } + + for edge := range pipeline.edges { + if edge.Op != pb.DirectedEdge_DEL && !schemaExists { + return errors.Errorf("runMutation: Unable to find schema for %s", edge.Attr) + } + + if err := ValidateAndConvert(edge, &su); err != nil { + return err + } + + uid := edge.Entity + pl, exists := mutations[uid] + if !exists { + pl = newMutableLayer() + pl.currentEntries = &pb.PostingList{} + } + + mpost := NewPosting(edge) + mpost.StartTs = mp.txn.StartTs + if mpost.PostingType != pb.Posting_REF { + edge.ValueId = FingerprintEdge(edge) + mpost.Uid = edge.ValueId + } + + pl.insertPosting(mpost, false) + mutations[uid] = pl + + if rawIndexPostings != nil { + rp, ok := rawIndexPostings[uid] + if !ok { + rp = &pb.PostingList{} + rawIndexPostings[uid] = rp + } + rp.Postings = append(rp.Postings, mpost) + } + } + + postings := make(map[uint64]*pb.PostingList, 1000) + for uid, pl := range mutations { + postings[uid] = pl.currentEntries + } + + if info.reverse { + if err := mp.ProcessReverse(ctx, pipeline, &postings, info); err != nil { + return err + } + } + + if info.index { + if err := mp.handleOldDeleteForList(pipeline, rawIndexPostings, info); err != nil { + return err + } + if err := mp.InsertTokenizerIndexes(ctx, pipeline, &rawIndexPostings, info); err != nil { + return err + } + } + + if info.count { + return mp.ProcessCount(ctx, pipeline, &postings, info, true, false) + } + + dataKey := x.DataKey(pipeline.attr, 0) + baseKey := string(dataKey[:len(dataKey)-8]) // Avoid repeated conversion + + for uid, pl := range postings { + if len(pl.Postings) == 0 { + continue + } + + binary.BigEndian.PutUint64(dataKey[len(dataKey)-8:], uid) + if newPl, err := mp.txn.AddDelta(baseKey+string(dataKey[len(dataKey)-8:]), pl, info.isUid, true); err != nil { + return err + } else { + if !info.noConflict { + mp.txn.addConflictKeyWithUid(dataKey, newPl, info.hasUpsert, info.noConflict) + } + } + } + + return nil +} + +func findSingleValueInPostingList(pb *pb.PostingList) *pb.Posting { + if pb == nil { + return nil + } + for _, p := range pb.Postings { + if p.Op == Set { + return p + } + } + return nil +} + +func (mp *MutationPipeline) ProcessReverse(ctx context.Context, pipeline *PredicatePipeline, postings *map[uint64]*pb.PostingList, info predicateInfo) error { + key := x.ReverseKey(pipeline.attr, 0) + edge := &pb.DirectedEdge{ + Attr: pipeline.attr, + } + reverseredMap := make(map[uint64]*pb.PostingList, 1000) + for uid, postingList := range *postings { + for _, posting := range postingList.Postings { + postingList, ok := reverseredMap[posting.Uid] + if !ok { + postingList = &pb.PostingList{} + } + edge.Entity = posting.Uid + edge.ValueId = uid + edge.ValueType = posting.ValType + edge.Op = GetPostingOp(posting.Op) + edge.Facets = posting.Facets + + postingList.Postings = append(postingList.Postings, makePostingFromEdge(mp.txn.StartTs, edge)) + reverseredMap[posting.Uid] = postingList + } + } + + if info.count { + newInfo := predicateInfo{ + isList: true, + index: info.index, + reverse: info.reverse, + count: info.count, + noConflict: info.noConflict, + hasUpsert: info.hasUpsert, + } + return mp.ProcessCount(ctx, pipeline, &reverseredMap, newInfo, true, true) + } + + for uid, pl := range reverseredMap { + if len(pl.Postings) == 0 { + continue + } + binary.BigEndian.PutUint64(key[len(key)-8:], uid) + if newPl, err := mp.txn.AddDelta(string(key), pl, true, true); err != nil { + return err + } else { + mp.txn.addConflictKeyWithUid(key, newPl, info.hasUpsert, info.noConflict) + } + } + + return nil +} + +func makePostingFromEdge(startTs uint64, edge *pb.DirectedEdge) *pb.Posting { + mpost := NewPosting(edge) + mpost.StartTs = startTs + if mpost.PostingType != pb.Posting_REF { + edge.ValueId = FingerprintEdge(edge) + mpost.Uid = edge.ValueId + } + return mpost +} + +func (mp *MutationPipeline) handleOldDeleteForSingle(pipeline *PredicatePipeline, postings map[uint64]*pb.PostingList) error { + edge := &pb.DirectedEdge{ + Attr: pipeline.attr, + } + + dataKey := x.DataKey(pipeline.attr, 0) + + for uid, postingList := range postings { + currValue := findSingleValueInPostingList(postingList) + if currValue == nil { + continue + } + + binary.BigEndian.PutUint64(dataKey[len(dataKey)-8:], uid) + list, err := mp.txn.GetScalarList(dataKey) + if err != nil { + return err + } + + oldValList, err := list.StaticValue(mp.txn.StartTs) + if err != nil { + return err + } + + oldVal := findSingleValueInPostingList(oldValList) + if oldVal == nil { + continue + } + + // For uid (REF) postings, Value is always nil — the target uid lives in + // posting.Uid. Compare Uid directly; fall back to Value comparison for scalars. + // + // When the new value matches the committed value, skip emitting the + // synthetic DEL-of-old-value (there is nothing to delete). Critically, + // do NOT clear postings[uid]: the SET must still flow through + // InsertTokenizerIndexes / ProcessReverse so a newly-added index or + // reverse edge can be populated for the (unchanged) value. Clearing + // here is what caused TestSchemaMutationIndexRemove to leave the term + // index empty after a same-value SET following an @index alter. + if currValue.PostingType == pb.Posting_REF && oldVal.PostingType == pb.Posting_REF { + if oldVal.Uid == currValue.Uid { + continue + } + } else if string(oldVal.Value) == string(currValue.Value) { + continue + } + + edge.Op = pb.DirectedEdge_DEL + edge.Value = oldVal.Value + edge.ValueType = oldVal.ValType + edge.ValueId = oldVal.Uid + + mpost := makePostingFromEdge(mp.txn.StartTs, edge) + postingList.Postings = append(postingList.Postings, mpost) + postings[uid] = postingList + } + + return nil +} + +// handleOldDeleteForList is the ProcessList counterpart of +// handleOldDeleteForSingle: for each scalar @lang (uid, fingerprint(lang)) +// being mutated, read the committed posting and append a synthetic DEL of the +// old value so InsertTokenizerIndexes can emit the corresponding index DEL. +// Skipped for list-typed predicates because SET on a list appends rather than +// replaces. +func (mp *MutationPipeline) handleOldDeleteForList(pipeline *PredicatePipeline, + rawPostings map[uint64]*pb.PostingList, info predicateInfo) error { + if rawPostings == nil || info.isList { + return nil + } + for uid, pl := range rawPostings { + // REF (uid) postings have no prior scalar value to remove. + seenFps := make(map[uint64]struct{}) + for _, p := range pl.Postings { + if p.PostingType != pb.Posting_REF { + seenFps[p.Uid] = struct{}{} + } + } + if len(seenFps) == 0 { + continue + } + + l, err := mp.txn.Get(x.DataKey(pipeline.attr, uid)) + if err != nil { + return err + } + if l == nil { + continue + } + + err = l.Iterate(mp.txn.StartTs, 0, func(p *pb.Posting) error { + if _, ok := seenFps[p.Uid]; !ok { + return nil + } + if p.Op == Del { + return nil + } + pl.Postings = append(pl.Postings, &pb.Posting{ + Uid: p.Uid, + Value: append([]byte(nil), p.Value...), + ValType: p.ValType, + LangTag: append([]byte(nil), p.LangTag...), + Op: Del, + StartTs: mp.txn.StartTs, + PostingType: p.PostingType, + }) + return nil + }) + if err != nil { + return err + } + } + return nil +} + +func (txn *Txn) addConflictKeyWithUid(key []byte, pl *pb.PostingList, hasUpsert bool, hasNoConflict bool) { + if hasNoConflict { + return + } + txn.Lock() + defer txn.Unlock() + if txn.conflicts == nil { + txn.conflicts = make(map[uint64]struct{}) + } + keyHash := farm.Fingerprint64(key) + if hasUpsert { + txn.conflicts[keyHash] = struct{}{} + return + } + for _, post := range pl.Postings { + txn.conflicts[keyHash^post.Uid] = struct{}{} + } +} + +func (mp *MutationPipeline) ProcessCount(ctx context.Context, pipeline *PredicatePipeline, postings *map[uint64]*pb.PostingList, info predicateInfo, isListEdge bool, isReverseEdge bool) error { + dataKey := x.DataKey(pipeline.attr, 0) + if isReverseEdge { + dataKey = x.ReverseKey(pipeline.attr, 0) + } + edge := pb.DirectedEdge{ + Attr: pipeline.attr, + } + + countMap := make(map[int]*pb.PostingList, 2*len(*postings)) + + insertEdgeCount := func(count int) { + c, ok := countMap[count] + if !ok { + c = &pb.PostingList{} + countMap[count] = c + } + c.Postings = append(c.Postings, makePostingFromEdge(mp.txn.StartTs, &edge)) + countMap[count] = c + } + + for uid, postingList := range *postings { + binary.BigEndian.PutUint64(dataKey[len(dataKey)-8:], uid) + list, err := mp.txn.Get(dataKey) + if err != nil { + return err + } + + list.Lock() + prevCount := list.GetLength(mp.txn.StartTs) + + // For scalar (non-list) predicates, handleOldDeleteForSingle may have + // appended a synthetic Del-of-old-value alongside the user's Set, so + // that InsertTokenizerIndexes / ProcessReverse / count diffing can see + // the prior value. The synthetic Del must NOT be applied to the data + // list: scalar value postings all share Uid == math.MaxUint64 + // (fingerprintEdge returns MaxUint64 for non-Lang scalars), and + // updateMutationLayer in singleUidUpdate mode would overwrite the + // just-inserted Set with [DeleteAll] and drop the new value entirely. + // A user-initiated Del (no accompanying Set) must still be applied. + skipSyntheticDel := false + if !isListEdge { + hasSet := false + for _, post := range postingList.Postings { + if post.Op == Set || post.Op == Ovr { + hasSet = true + break + } + } + skipSyntheticDel = hasSet + } + + for _, post := range postingList.Postings { + if skipSyntheticDel && post.Op == Del { + continue + } + found, _, _ := list.findPosting(post.StartTs, post.Uid) + if found { + if post.Op == Set && isListEdge { + post.Op = Ovr + } + } else { + if post.Op == Del { + continue + } + } + + if err := list.updateMutationLayer(post, !isListEdge, true); err != nil { + return err + } + } + + newCount := list.GetLength(mp.txn.StartTs) + updated := list.mutationMap.currentEntries != nil + list.Unlock() + + if updated { + if !isListEdge { + if !info.noConflict { + mp.txn.addConflictKey(farm.Fingerprint64(dataKey)) + } + } else { + mp.txn.addConflictKeyWithUid(dataKey, postingList, info.hasUpsert, info.noConflict) + } + } + + if newCount == prevCount { + continue + } + + edge.ValueId = uid + edge.Op = pb.DirectedEdge_DEL + if prevCount > 0 { + insertEdgeCount(prevCount) + } + edge.Op = pb.DirectedEdge_SET + if newCount > 0 { + insertEdgeCount(newCount) + } + } + + for c, pl := range countMap { + ck := x.CountKey(pipeline.attr, uint32(c), isReverseEdge) + if newPl, err := mp.txn.AddDelta(string(ck), pl, true, true); err != nil { + return err + } else { + mp.txn.addConflictKeyWithUid(ck, newPl, info.hasUpsert, info.noConflict) + } + } + + return nil +} + +// ProcessSingle handles scalar (non-list) predicates without a language tag. +// Phase 1: drain the edges channel into uid→PostingList map with no Badger reads — +// +// multiple SET/DEL edges for the same uid are folded in-memory (last-SET wins). +// +// Phase 2: if indexed/reversed/counted, call handleOldDeleteForSingle to fetch the +// +// current disk value for each uid once (O(distinct_uids) reads) so index/reverse +// Del entries for the old value can be generated correctly. +// +// Phase 3: batch-write all accumulated deltas via txn.AddDelta in one loop. +// Contrast with legacy runMutation: every edge caused a GetScalarList (one Badger +// read) plus AddMutationWithIndex (another read to fetch old value for index Del). +func (mp *MutationPipeline) ProcessSingle(ctx context.Context, pipeline *PredicatePipeline, info predicateInfo) error { + su, schemaExists := schema.State().Get(ctx, pipeline.attr) + + postings := make(map[uint64]*pb.PostingList, 1000) + + dataKey := x.DataKey(pipeline.attr, 0) + insertDeleteAllEdge := !(info.index || info.reverse || info.count) // nolint + + // For uid postings the target lives in Uid/ValueId; Value is nil so a + // bytewise comparison would always succeed and DEL would wrongly overwrite + // an accumulated SET in the same batch. + sameTarget := func(oldVal *pb.Posting, edge *pb.DirectedEdge) bool { + if info.isUid { + return oldVal.Uid == edge.ValueId + } + return string(oldVal.Value) == string(edge.Value) + } + + for edge := range pipeline.edges { + if edge.Op != pb.DirectedEdge_DEL && !schemaExists { + return errors.Errorf("runMutation: Unable to find schema for %s", edge.Attr) + } + + if err := ValidateAndConvert(edge, &su); err != nil { + return err + } + + // oldVal is reset per edge so a stale value from a previous iteration + // can't bleed into the nil-guarded branch below. + var oldVal *pb.Posting + uid := edge.Entity + pl, exists := postings[uid] + + setPosting := func() { + mpost := makePostingFromEdge(mp.txn.StartTs, edge) + if len(pl.Postings) == 0 { + if insertDeleteAllEdge { + pl = &pb.PostingList{ + Postings: []*pb.Posting{createDeleteAllPosting(), mpost}, + } + } else { + pl = &pb.PostingList{ + Postings: []*pb.Posting{mpost}, + } + } + } else { + if pl.Postings[len(pl.Postings)-1].Op == Set { + pl.Postings[len(pl.Postings)-1] = mpost + } else { + pl.Postings = append(pl.Postings, mpost) + } + } + postings[uid] = pl + } + + if exists { + if edge.Op == pb.DirectedEdge_DEL { + // findSingleValueInPostingList returns nil when the + // accumulated postings for this uid hold only Del entries + // (no Set), which happens when the same uid receives + // multiple Del edges in one batch (e.g. GraphQL + // deleteTask cleanup deleting many predicates per entity). + oldVal = findSingleValueInPostingList(pl) + if oldVal != nil && sameTarget(oldVal, edge) { + setPosting() + } + } else { + setPosting() + } + continue + } + + pl = &pb.PostingList{} + postings[uid] = pl + + if edge.Op == pb.DirectedEdge_DEL { + binary.BigEndian.PutUint64(dataKey[len(dataKey)-8:], uid) + list, err := mp.txn.GetScalarList(dataKey) + if err != nil { + return err + } + if list != nil { + l, err := list.StaticValue(mp.txn.StartTs) + if err != nil { + return err + } + oldVal = findSingleValueInPostingList(l) + } + if oldVal != nil { + if sameTarget(oldVal, edge) { + setPosting() + } + } + } else { + setPosting() + } + } + + if info.index || info.reverse || info.count { + if err := mp.handleOldDeleteForSingle(pipeline, postings); err != nil { + return err + } + } + + if info.index { + if err := mp.InsertTokenizerIndexes(ctx, pipeline, &postings, info); err != nil { + return err + } + } + + if info.reverse { + if err := mp.ProcessReverse(ctx, pipeline, &postings, info); err != nil { + return err + } + } + + if info.count { + // Count should take care of updating the posting list + return mp.ProcessCount(ctx, pipeline, &postings, info, false, false) + } + + baseKey := string(dataKey[:len(dataKey)-8]) // Avoid repeated conversion + // handleOldDeleteForSingle may have appended a synthetic Del-of-old-value + // alongside the user's Set so InsertTokenizerIndexes / ProcessReverse can + // emit index/reverse Del entries for the prior value. The synthetic Del + // must NOT be written to the scalar data list: scalar value postings all + // share Uid == math.MaxUint64, and at read time pickPostings sorts the two + // equal-ts postings non-deterministically (Go's sort.Slice is unstable), + // while setMutationAfterCommit's committedUids[mpost.Uid] = mpost + // overwrites in append order. Either way Del can clobber the new Set, so + // the data list reads as "no value." Strip only for scalar (@index) predicates. + // For uid (@reverse) predicates the synthetic Del has a distinct Uid (the old + // target uid) so there is no collision — it must stay so the old forward edge + // is removed, mirroring the legacy isSingleUidUpdate explicit DEL. + stripSyntheticDel := info.index + + for uid, pl := range postings { + // An empty PostingList means nothing was accumulated for this uid (e.g. + // a DEL whose value did not match the committed value, or a same-value + // SET-after-handleOldDeleteForSingle no-op). Writing an empty delta + // would call AddDelta → setMutation on the cached List with empty bytes, + // which resets the mutable layer's currentEntries to empty for this + // txn's startTs. A subsequent read through the same cached List then + // surfaces an empty data list. CommitToDisk skips empty deltas at the + // badger level, but the in-memory side effect on the cached List has + // already corrupted any concurrent reader holding that List. Skip the + // no-op uid entirely — matches the legacy per-edge path which simply + // does nothing when a DEL value does not match committed. + if len(pl.Postings) == 0 { + continue + } + + binary.BigEndian.PutUint64(dataKey[len(dataKey)-8:], uid) + key := baseKey + string(dataKey[len(dataKey)-8:]) + + if !info.noConflict { + mp.txn.addConflictKey(farm.Fingerprint64([]byte(key))) + } + + writePl := pl + if stripSyntheticDel && len(pl.Postings) > 1 { + hasSet := false + for _, post := range pl.Postings { + if post.Op == Set || post.Op == Ovr { + hasSet = true + break + } + } + if hasSet { + filtered := &pb.PostingList{} + for _, post := range pl.Postings { + if post.Op == Del { + continue + } + filtered.Postings = append(filtered.Postings, post) + } + writePl = filtered + } + } + + if _, err := mp.txn.AddDelta(key, writePl, false, false); err != nil { + return err + } + } + + return nil +} + +func runMutation(ctx context.Context, edge *pb.DirectedEdge, txn *Txn) error { + ctx = schema.GetWriteContext(ctx) + + // We shouldn't check whether this Alpha serves this predicate or not. Membership information + // isn't consistent across the entire cluster. We should just apply whatever is given to us. + su, ok := schema.State().Get(ctx, edge.Attr) + if edge.Op != pb.DirectedEdge_DEL { + if !ok { + return errors.Errorf("runMutation: Unable to find schema for %s", edge.Attr) + } + } + + key := x.DataKey(edge.Attr, edge.Entity) + // The following is a performance optimization which allows us to not read a posting list from + // disk. We calculate this based on how AddMutationWithIndex works. The general idea is that if + // we're not using the read posting list, we don't need to retrieve it. We need the posting list + // if we're doing count index or delete operation. For scalar predicates, we just get the last item merged. + // In other cases, we can just create a posting list facade in memory and use it to store the delta in Badger. + // Later, the rollup operation would consolidate all these deltas into a posting list. + isList := su.GetList() + var getFn func(key []byte) (*List, error) + switch { + case len(edge.Lang) == 0 && !isList: + // Scalar Predicates, without lang + getFn = txn.GetScalarList + case len(edge.Lang) > 0 || su.GetCount(): + // Language or Count Index + getFn = txn.Get + case edge.Op == pb.DirectedEdge_DEL: + // Covers various delete cases to keep things simple. + getFn = txn.Get + default: + // Only count index needs to be read. For other indexes on list, we don't need to read any data. + // For indexes on scalar prediactes, only the last element needs to be left. + // Delete cases covered above. + getFn = txn.GetFromDelta + } + + plist, err := getFn(key) + if err != nil { + return err + } + return plist.AddMutationWithIndex(ctx, edge, txn) +} + +// ProcessPredicate is the per-predicate goroutine entry point. It reads the schema once +// for the predicate and routes to the appropriate processor: +// +// vector index → ProcessVectorIndex (serial; HNSW cross-key writes are not concurrent-safe) +// list/lang → ProcessList (batched uid-map accumulation) +// scalar → ProcessSingle (batched uid-map accumulation) +// unknown schema → runMutation fallback (legacy, edge-by-edge) +// +// The fallback at the bottom of the function handles edges that are neither list nor +// scalar — these are drained from the channel after ProcessList/ProcessSingle return, +// though in practice ProcessList/ProcessSingle consume all edges so the channel is +// already empty. +func (mp *MutationPipeline) ProcessPredicate(ctx context.Context, pipeline *PredicatePipeline) error { + defer pipeline.close() + ctx = schema.GetWriteContext(ctx) + + // We shouldn't check whether this Alpha serves this predicate or not. Membership information + // isn't consistent across the entire cluster. We should just apply whatever is given to us. + su, ok := schema.State().Get(ctx, pipeline.attr) + info := predicateInfo{} + runForVectorIndex := false + + if ok { + info.index = schema.State().IsIndexed(ctx, pipeline.attr) + info.count = schema.State().HasCount(ctx, pipeline.attr) + info.reverse = schema.State().IsReversed(ctx, pipeline.attr) + info.noConflict = schema.State().HasNoConflict(pipeline.attr) + info.hasUpsert = schema.State().HasUpsert(pipeline.attr) + info.isList = schema.State().IsList(pipeline.attr) + info.isUid = su.ValueType == pb.Posting_UID + factorySpecs, err := schema.State().FactoryCreateSpec(ctx, pipeline.attr) + if err != nil { + return err + } + if len(factorySpecs) > 0 { + runForVectorIndex = true + } + } + + if runForVectorIndex { + return mp.ProcessVectorIndex(ctx, pipeline, info) + } + + runListFn := false + + if ok { + if info.isList || su.Lang { + runListFn = true + } + } + + info.isSingleEdge = !runListFn + + if runListFn { + if err := mp.ProcessList(ctx, pipeline, info); err != nil { + return err + } + } + + if ok && !runListFn { + if err := mp.ProcessSingle(ctx, pipeline, info); err != nil { + return err + } + } + + for edge := range pipeline.edges { + if err := runMutation(ctx, edge, mp.txn); err != nil { + return err + } + } + + return nil +} + +func isStarAll(v []byte) bool { + return bytes.Equal(v, []byte(x.Star)) +} + +func ValidateAndConvert(edge *pb.DirectedEdge, su *pb.SchemaUpdate) error { + if types.TypeID(edge.ValueType) == types.DefaultID && isStarAll(edge.Value) { + return nil + } + + storageType := TypeID(edge) + schemaType := types.TypeID(su.ValueType) + + // type checks + switch { + case edge.Lang != "" && !su.GetLang(): + return errors.Errorf("Attr: [%v] should have @lang directive in schema to mutate edge: [%v]", + x.ParseAttr(edge.Attr), edge) + + case !schemaType.IsScalar() && !storageType.IsScalar(): + return nil + + case !schemaType.IsScalar() && storageType.IsScalar(): + return errors.Errorf("Input for predicate %q of type uid is scalar. Edge: %v", + x.ParseAttr(edge.Attr), edge) + + case schemaType.IsScalar() && !storageType.IsScalar(): + return errors.Errorf("Input for predicate %q of type scalar is uid. Edge: %v", + x.ParseAttr(edge.Attr), edge) + + case schemaType == types.TypeID(pb.Posting_VFLOAT): + if !(storageType == types.TypeID(pb.Posting_VFLOAT) || storageType == types.TypeID(pb.Posting_STRING) || //nolint + storageType == types.TypeID(pb.Posting_DEFAULT)) { + return errors.Errorf("Input for predicate %q of type vector is not vector."+ + " Did you forget to add quotes before []?. Edge: %v", x.ParseAttr(edge.Attr), edge) + } + + // The suggested storage type matches the schema, OK! (Nothing to do ...) + case storageType == schemaType && schemaType != types.DefaultID: + return nil + + // We accept the storage type iff we don't have a schema type and a storage type is specified. + case schemaType == types.DefaultID: + schemaType = storageType + } + + var ( + dst types.Val + err error + ) + + src := types.Val{Tid: types.TypeID(edge.ValueType), Value: edge.Value} + // check compatibility of schema type and storage type + // The goal is to convert value on edge to value type defined by schema. + if dst, err = types.Convert(src, schemaType); err != nil { + return err + } + + // convert to schema type + b := types.ValueForType(types.BinaryID) + if err = types.Marshal(dst, &b); err != nil { + return err + } + + if x.WorkerConfig.AclEnabled && x.ParseAttr(edge.GetAttr()) == "dgraph.rule.permission" { + perm, ok := dst.Value.(int64) + if !ok { + return errors.Errorf("Value for predicate should be of type int") + } + if perm < 0 || perm > 7 { + return errors.Errorf("Can't set to %d, Value for this"+ + " predicate should be between 0 and 7", perm) + } + } + + // TODO: Figure out why this is Enum. It really seems like an odd choice -- rather than + // specifying it as the same type as presented in su. + edge.ValueType = schemaType.Enum() + var ok bool + edge.Value, ok = b.Value.([]byte) + if !ok { + return errors.Errorf("failure to convert edge type: '%+v' to schema type: '%+v'", + storageType, schemaType) + } + + return nil +} + +// Process is the entry point for the mutation pipeline. It fans out one goroutine per +// predicate so mutations for different predicates are parallelized, while all edges +// for the same predicate are serialized through a single channel to a single goroutine. +// This serialization per-predicate is intentional: ProcessSingle and ProcessList both +// build a uid-keyed map that assumes they own all writes for their predicate, and the +// index/reverse/count secondary-key updates must see the complete accumulated state to +// produce correct deltas. Running two goroutines for the same predicate would cause +// lost updates and incorrect index deltas. +// +// Edge shape entering this function: +// +// edge.Entity — subject UID (e.g. 0x1234) +// edge.Attr — predicate (e.g. "name", "age", "friend") +// edge.Value — object bytes for scalar predicates (e.g. []byte("Alice")) +// edge.ValueId — object UID for uid predicates (e.g. 0x5678) +// edge.Op — SET or DEL +// edge.Lang — language tag (e.g. "en") or empty +// edge.Facets — optional facet list +// +// Star-delete edges (entity=0, value="*") are handled inline before the fan-out +// because they must run serially with respect to the data list they are clearing. +func (mp *MutationPipeline) Process(ctx context.Context, edges []*pb.DirectedEdge) error { + // handleDeleteAll calls schema.State().IsIndexed/IsReversed/HasCount, all of + // which only consult the pending mutSchema (the new schema with the index + // being built) when the context carries the isWrite flag. The legacy path + // in runMutation already calls schema.GetWriteContext at the top; the + // pipeline must do the same here, otherwise during a background index + // build a star-delete won't generate the index Del entries for the prior + // value, leaving stale uids in the index permanently. + ctx = schema.GetWriteContext(ctx) + predicates := map[string]*PredicatePipeline{} + var wg sync.WaitGroup + numWg := 0 + eg, egCtx := errgroup.WithContext(ctx) + var sendErr error +sendLoop: + for _, edge := range edges { + if edge.Op == pb.DirectedEdge_DEL && string(edge.Value) == x.Star { + l, err := mp.txn.Get(x.DataKey(edge.Attr, edge.Entity)) + if err != nil { + sendErr = err + break sendLoop + } + if err = l.handleDeleteAll(ctx, edge, mp.txn); err != nil { + sendErr = err + break sendLoop + } + continue + } + pred, ok := predicates[edge.Attr] + if !ok { + pred = &PredicatePipeline{ + attr: edge.Attr, + edges: make(chan *pb.DirectedEdge, 1000), + wg: &wg, + } + predicates[edge.Attr] = pred + wg.Add(1) + numWg += 1 + // Capture pred by passing it as a parameter to the closure + eg.Go(func(p *PredicatePipeline) func() error { + return func() error { + return mp.ProcessPredicate(egCtx, p) + } + }(pred)) + } + // Guard the send: if a predicate goroutine returns early on error, the + // channel reader is gone and a bare send blocks forever once the buffer fills. + select { + case pred.edges <- edge: + case <-egCtx.Done(): + sendErr = egCtx.Err() + break sendLoop + } + } + for _, pred := range predicates { + close(pred.edges) + } + if numWg == 0 { + return sendErr + } + // Wait for all predicate processors; returns the first goroutine error, + // which is the real root cause when sendErr is just context cancellation. + if err := eg.Wait(); err != nil { + return err + } + return sendErr +} func (txn *Txn) addIndexMutations(ctx context.Context, info *indexMutationInfo) ([]*pb.DirectedEdge, error) { if info.tokenizers == nil { @@ -951,7 +2101,7 @@ func (r *rebuilder) Run(ctx context.Context) error { // txn.cache.Lock() is not required because we are the only one making changes to txn. kvs := make([]*bpb.KV, 0) - for key, data := range streamTxn.cache.deltas { + if err := streamTxn.cache.deltas.IterateBytes(func(key string, data []byte) error { version := atomic.AddUint64(&counter, 1) kv := bpb.KV{ Key: []byte(key), @@ -960,7 +2110,11 @@ func (r *rebuilder) Run(ctx context.Context) error { Version: version, } kvs = append(kvs, &kv) + return nil + }); err != nil { + return nil, err } + txns[threadId] = NewTxn(r.startTs) return &bpb.KVList{Kv: kvs}, nil } @@ -1009,7 +2163,7 @@ func (r *rebuilder) Run(ctx context.Context) error { // Convert data into deltas. streamTxn.Update() // txn.cache.Lock() is not required because we are the only one making changes to txn. - for key, data := range streamTxn.cache.deltas { + if err := streamTxn.cache.deltas.IterateBytes(func(key string, data []byte) error { version := atomic.AddUint64(&counter, 1) kv := bpb.KV{ Key: []byte(key), @@ -1018,6 +2172,9 @@ func (r *rebuilder) Run(ctx context.Context) error { Version: version, } kvs = append(kvs, &kv) + return nil + }); err != nil { + return nil, err } txns[threadId] = NewTxn(r.startTs) diff --git a/posting/index_test.go b/posting/index_test.go index ff481f7ace5..ac137450f7e 100644 --- a/posting/index_test.go +++ b/posting/index_test.go @@ -139,19 +139,44 @@ func addMutation(t *testing.T, l *List, edge *pb.DirectedEdge, op uint32, } txn := Oracle().RegisterStartTs(startTs) txn.cache.SetIfAbsent(string(l.key), l) - if index { - require.NoError(t, l.AddMutationWithIndex(context.Background(), edge, txn)) - } else { - require.NoError(t, l.addMutation(context.Background(), txn, edge)) + + // Production creates a schema entry for any new predicate in applyMutations before the + // mutation runs; mirror that here so tests that never declare one behave the same way. + if _, ok := schema.State().Get(context.Background(), edge.Attr); !ok { + su := &pb.SchemaUpdate{Predicate: edge.Attr} + if edge.ValueId != 0 && len(edge.Value) == 0 { + su.ValueType = pb.Posting_UID + su.List = true + } else { + su.ValueType = edge.ValueType + if su.ValueType == pb.Posting_DEFAULT { + su.ValueType = pb.Posting_STRING + } + } + schema.State().Set(edge.Attr, su) } + mp := NewMutationPipeline(txn) + err := mp.Process(context.Background(), []*pb.DirectedEdge{edge}) + require.NoError(t, err) txn.Update() txn.UpdateCachedKeys(commitTs) writer := NewTxnWriter(pstore) require.NoError(t, txn.CommitToDisk(writer, commitTs)) require.NoError(t, writer.Flush()) + newTxn := Oracle().RegisterStartTs(commitTs + 1) + l1, err := newTxn.Get(l.key) + require.NoError(t, err) + *l = *l1 //nolint } +const schemaPreVal = ` + name: string . + name2: string . + dob: dateTime . + friend: [uid] . +` + const schemaVal = ` name: string @index(term) . name2: string @index(term) . @@ -263,6 +288,9 @@ func addEdgeToUID(t *testing.T, attr string, src uint64, func TestCountReverseIndexWithData(t *testing.T) { require.NoError(t, pstore.DropAll()) MemLayerInstance.clear() + preIndex := "testcount: [uid] ." + require.NoError(t, schema.ParseBytes([]byte(preIndex), 1)) + indexNameCountVal := "testcount: [uid] @count @reverse ." attr := x.AttrInRootNamespace("testcount") @@ -298,6 +326,9 @@ func TestCountReverseIndexWithData(t *testing.T) { func TestCountReverseIndexEmptyPosting(t *testing.T) { require.NoError(t, pstore.DropAll()) MemLayerInstance.clear() + preIndex := "testcount: [uid] ." + require.NoError(t, schema.ParseBytes([]byte(preIndex), 1)) + indexNameCountVal := "testcount: [uid] @count @reverse ." attr := x.AttrInRootNamespace("testcount") @@ -330,6 +361,8 @@ func TestCountReverseIndexEmptyPosting(t *testing.T) { } func TestRebuildTokIndex(t *testing.T) { + require.NoError(t, schema.ParseBytes([]byte(schemaPreVal), 1)) + addEdgeToValue(t, x.AttrInRootNamespace("name2"), 91, "Michonne", uint64(1), uint64(2)) addEdgeToValue(t, x.AttrInRootNamespace("name2"), 92, "David", uint64(3), uint64(4)) diff --git a/posting/list.go b/posting/list.go index 3fcb24e9597..c3f4fd62b0d 100644 --- a/posting/list.go +++ b/posting/list.go @@ -160,6 +160,33 @@ func (mm *MutableLayer) setCurrentEntries(ts uint64, pl *pb.PostingList) { x.AssertTruef(mm.readTs == ts, "List object reused for a different transaction %d %d", mm.readTs, ts) } + // If currentEntries already contained a deleteAll (e.g. set inline by + // handleDeleteAll before the pipeline writes its accumulated postings), + // preserve it so a subsequent same-batch SET doesn't lose the deleteAll + // signal. The legacy path naturally avoids this because it appends via + // insertPosting instead of overwriting. + if mm.currentEntries != nil { + var existingDeleteAll *pb.Posting + for _, p := range mm.currentEntries.Postings { + if hasDeleteAll(p) { + existingDeleteAll = p + break + } + } + if existingDeleteAll != nil { + alreadyHas := false + for _, p := range pl.Postings { + if hasDeleteAll(p) { + alreadyHas = true + break + } + } + if !alreadyHas { + pl.Postings = append([]*pb.Posting{existingDeleteAll}, pl.Postings...) + } + } + } + mm.readTs = ts mm.currentEntries = pl clear(mm.currentUids) @@ -406,10 +433,11 @@ func (mm *MutableLayer) print() string { if mm == nil { return "" } - return fmt.Sprintf("Committed List: %+v Proposed list: %+v Delete all marker: %d \n", + return fmt.Sprintf("Committed List: %+v Proposed list: %+v Delete all marker: %d. Count: %d \n", mm.committedEntries, mm.currentEntries, - mm.deleteAllMarker) + mm.deleteAllMarker, + mm.length) } // printShort returns a summary of the mutable layer with the entry counts but not the entries themselves. Use it in @@ -425,6 +453,10 @@ func (mm *MutableLayer) printShort() string { // Print dumps the entire list, posting values included. It is meant for interactive debugging only. Anything that // ends up in a log line or an error message should use debugString() instead. func (l *List) Print() string { + if l.plist.Pack != nil { + uids := codec.Decode(l.plist.Pack, 0) + return fmt.Sprintf("minTs: %d, committed uids: %+v, mutationMap: %s", l.minTs, uids, l.mutationMap.print()) + } return fmt.Sprintf("minTs: %d, plist: %+v, mutationMap: %s", l.minTs, l.plist, l.mutationMap.print()) } @@ -733,6 +765,53 @@ type ListOptions struct { First int } +func NewPostingExisting(p *pb.Posting, t *pb.DirectedEdge) { + var op uint32 + switch t.Op { + case pb.DirectedEdge_SET: + op = Set + case pb.DirectedEdge_OVR: + op = Ovr + case pb.DirectedEdge_DEL: + op = Del + default: + x.Fatalf("Unhandled operation: %+v", t) + } + + var postingType pb.Posting_PostingType + switch { + case len(t.Lang) > 0: + postingType = pb.Posting_VALUE_LANG + case t.ValueId == 0: + postingType = pb.Posting_VALUE + default: + postingType = pb.Posting_REF + } + + p.Uid = t.ValueId + p.Value = t.Value + p.ValType = t.ValueType + p.PostingType = postingType + p.LangTag = []byte(t.Lang) + p.Op = op + p.Facets = t.Facets +} + +func GetPostingOp(top uint32) pb.DirectedEdge_Op { + var op pb.DirectedEdge_Op + switch top { + case Set: + op = pb.DirectedEdge_SET + case Del: + op = pb.DirectedEdge_DEL + case Ovr: + op = pb.DirectedEdge_OVR + default: + x.Fatalf("Unhandled operation: %+v", top) + } + return op +} + // NewPosting takes the given edge and returns its equivalent representation as a posting. func NewPosting(t *pb.DirectedEdge) *pb.Posting { var op uint32 @@ -810,12 +889,12 @@ func (l *List) updateMutationLayer(mpost *pb.Posting, singleUidUpdate, hasCountI // The current value should be deleted in favor of this value. This needs to // be done because the fingerprint for the value is not math.MaxUint64 as is // the case with the rest of the scalar predicates. - newPlist := &pb.PostingList{} + newPlist := &pb.PostingList{ + Postings: []*pb.Posting{createDeleteAllPosting()}, + } if mpost.Op != Del { - // If we are setting a new value then we can just delete all the older values. - newPlist.Postings = append(newPlist.Postings, createDeleteAllPosting()) + newPlist.Postings = append(newPlist.Postings, mpost) } - newPlist.Postings = append(newPlist.Postings, mpost) l.mutationMap.setCurrentEntries(mpost.StartTs, newPlist) return nil } @@ -854,6 +933,10 @@ func fingerprintEdge(t *pb.DirectedEdge) uint64 { return id } +func FingerprintEdge(t *pb.DirectedEdge) uint64 { + return fingerprintEdge(t) +} + func (l *List) addMutation(ctx context.Context, txn *Txn, t *pb.DirectedEdge) error { l.Lock() defer l.Unlock() @@ -1069,7 +1152,10 @@ func (l *List) setMutationAfterCommit(startTs, commitTs uint64, pl *pb.PostingLi func (l *List) setMutation(startTs uint64, data []byte) { pl := new(pb.PostingList) x.Check(proto.Unmarshal(data, pl)) + l.setMutationWithPosting(startTs, pl) +} +func (l *List) setMutationWithPosting(startTs uint64, pl *pb.PostingList) { l.Lock() if l.mutationMap == nil { l.mutationMap = newMutableLayer() @@ -1136,6 +1222,13 @@ func (l *List) pickPostings(readTs uint64) (uint64, []*pb.Posting) { } return pi.Uid < pj.Uid }) + + if len(posts) > 0 { + if hasDeleteAll(posts[0]) { + posts = posts[1:] + } + } + return deleteAllMarker, posts } @@ -1284,6 +1377,11 @@ func (l *List) GetLength(readTs uint64) int { length += immutLen } + // pureLength := l.length(readTs, 0) + // if pureLength != length { + // panic(fmt.Sprintf("pure length != length %d %d %s", pureLength, length, l.Print())) + // } + return length } @@ -1493,7 +1591,6 @@ func (l *List) Rollup(alloc *z.Allocator, readTs uint64) ([]*bpb.KV, error) { return bytes.Compare(kvs[i].Key, kvs[j].Key) <= 0 }) - x.PrintRollup(out.plist, out.parts, l.key, kv.Version) x.VerifyPostingSplits(kvs, out.plist, out.parts, l.key) return kvs, nil } @@ -2129,6 +2226,18 @@ func (l *List) findStaticValue(readTs uint64) *pb.PostingList { if l.plist != nil && len(l.plist.Postings) > 0 { return l.plist } + if l.plist != nil && l.plist.Pack != nil { + uids := codec.Decode(l.plist.Pack, 0) + return &pb.PostingList{ + Postings: []*pb.Posting{ + { + Uid: uids[0], + ValType: pb.Posting_UID, + Op: Set, + }, + }, + } + } return nil } diff --git a/posting/list_test.go b/posting/list_test.go index 07cfaf1718c..ed15f277ef8 100644 --- a/posting/list_test.go +++ b/posting/list_test.go @@ -124,7 +124,6 @@ func TestGetSinglePosting(t *testing.T) { res, err := l.StaticValue(1) require.NoError(t, err) - fmt.Println(res, res == nil) require.Equal(t, res == nil, true) l.plist = create_pl(1, 1) diff --git a/posting/lists.go b/posting/lists.go index 572811d5889..4a85ffa3abf 100644 --- a/posting/lists.go +++ b/posting/lists.go @@ -19,6 +19,7 @@ import ( "github.com/dgraph-io/badger/v4" "github.com/dgraph-io/dgo/v250/protos/api" "github.com/dgraph-io/dgraph/v25/protos/pb" + "github.com/dgraph-io/dgraph/v25/types" "github.com/dgraph-io/dgraph/v25/x" "github.com/dgraph-io/ristretto/v2/z" ) @@ -66,10 +67,7 @@ type LocalCache struct { startTs uint64 commitTs uint64 - // The keys for these maps is a string representation of the Badger key for the posting list. - // deltas keep track of the updates made by txn. These must be kept around until written to disk - // during commit. - deltas map[string][]byte + deltas *Deltas // max committed timestamp of the read posting lists. maxVersions map[string]uint64 @@ -78,6 +76,141 @@ type LocalCache struct { plists map[string]*List } +// The keys for these maps is a string representation of the Badger key for the posting list. +// deltas keep track of the updates made by txn. These must be kept around until written to disk +// during commit. +type Deltas struct { + deltas *types.LockedShardedMap[string, []byte] + + // We genereate indexes for the posting lists all at once. Moving them from this map to deltas + // map is uneccessary. More data can be stored per predicate later on. + indexMap map[string]*types.LockedShardedMap[string, *pb.PostingList] +} + +func NewDeltas() *Deltas { + return &Deltas{ + deltas: types.NewLockedShardedMap[string, []byte](), + indexMap: map[string]*types.LockedShardedMap[string, *pb.PostingList]{}, + } +} + +// Call this function after taking a lock on the cache. +func (d *Deltas) GetIndexMapForPredicate(pred string) *types.LockedShardedMap[string, *pb.PostingList] { + val, ok := d.indexMap[pred] + if !ok { + d.indexMap[pred] = types.NewLockedShardedMap[string, *pb.PostingList]() + return d.indexMap[pred] + } + return val +} + +func (d *Deltas) Get(key string) (*pb.PostingList, bool) { + if d == nil { + return nil, false + } + pk, err := x.Parse([]byte(key)) + if err != nil { + return nil, false + } + + res := &pb.PostingList{} + + val, ok := d.deltas.Get(key) + if ok { + if err := proto.Unmarshal(val, res); err != nil { + return nil, false + } + } + + if indexMap, ok := d.indexMap[pk.Attr]; ok { + if value, ok1 := indexMap.Get(key); ok1 { + res.Postings = append(res.Postings, value.Postings...) + } + } + + return res, len(res.Postings) > 0 +} + +func (d *Deltas) GetBytes(key string) ([]byte, bool) { + if len(d.indexMap) == 0 { + return d.deltas.Get(key) + } + + pk, err := x.Parse([]byte(key)) + if err != nil { + return nil, false + } + + delta, deltaFound := d.deltas.Get(key) + + if indexMap, ok := d.indexMap[pk.Attr]; ok { + if value, ok1 := indexMap.Get(key); ok1 && deltaFound && len(value.Postings) > 0 { + res := &pb.PostingList{} + if err := proto.Unmarshal(delta, res); err != nil { + return nil, false + } + res.Postings = append(res.Postings, value.Postings...) + data, err := proto.Marshal(res) + if err != nil { + return nil, false + } + return data, true + } else if ok1 && len(value.Postings) > 0 { + data, err := proto.Marshal(value) + if err != nil { + return nil, false + } + return data, true + } + } + + return delta, deltaFound +} + +func (d *Deltas) AddToDeltas(key string, delta []byte) { + d.deltas.Set(key, delta) +} + +func (d *Deltas) IterateKeys(fn func(key string) error) error { + for _, v := range d.indexMap { + if err := v.Iterate(func(key string, value *pb.PostingList) error { + return fn(key) + }); err != nil { + return err + } + } + if err := d.deltas.Iterate(func(key string, value []byte) error { + return fn(key) + }); err != nil { + return err + } + return nil +} + +func (d *Deltas) IteratePostings(fn func(key string, value *pb.PostingList) error) error { + return d.IterateKeys(func(key string) error { + val, ok := d.Get(key) + if !ok { + return nil + } + return fn(key, val) + }) +} + +func (d *Deltas) IterateBytes(fn func(key string, value []byte) error) error { + return d.IterateKeys(func(key string) error { + val, ok := d.Get(key) + if !ok { + return nil + } + data, err := proto.Marshal(val) + if err != nil { + return err + } + return fn(key, data) + }) +} + // struct to implement LocalCache interface from vector-indexer // acts as wrapper for dgraph *LocalCache type viLocalCache struct { @@ -132,7 +265,7 @@ func NewViLocalCache(delegate *LocalCache) *viLocalCache { func NewLocalCache(startTs uint64) *LocalCache { return &LocalCache{ startTs: startTs, - deltas: make(map[string][]byte), + deltas: NewDeltas(), plists: make(map[string]*List), maxVersions: make(map[string]uint64), } @@ -264,6 +397,9 @@ func (lc *LocalCache) getInternal(key []byte, readFromDisk, readUids bool) (*Lis return getNew(key, pstore, lc.startTs, readUids) } if l, ok := lc.plists[skey]; ok { + if delta, ok := lc.deltas.Get(skey); ok && delta != nil { + l.setMutationWithPosting(lc.startTs, delta) + } return l, nil } return nil, nil @@ -291,10 +427,11 @@ func (lc *LocalCache) getInternal(key []byte, readFromDisk, readUids bool) (*Lis // If we just brought this posting list into memory and we already have a delta for it, let's // apply it before returning the list. lc.RLock() - if delta, ok := lc.deltas[skey]; ok && len(delta) > 0 { - pl.setMutation(lc.startTs, delta) + if delta, ok := lc.deltas.Get(skey); ok && delta != nil { + pl.setMutationWithPosting(lc.startTs, delta) } lc.RUnlock() + return lc.SetIfAbsent(skey, pl), nil } @@ -334,11 +471,9 @@ func (lc *LocalCache) GetSinglePosting(key []byte) (*pb.PostingList, error) { getListFromLocalCache := func() (*pb.PostingList, error) { lc.RLock() - pl := &pb.PostingList{} - if delta, ok := lc.deltas[string(key)]; ok && len(delta) > 0 { - err := proto.Unmarshal(delta, pl) + if delta, ok := lc.deltas.Get(string(key)); ok && delta != nil { lc.RUnlock() - return pl, err + return delta, nil } l := lc.plists[string(key)] @@ -373,9 +508,6 @@ func (lc *LocalCache) GetSinglePosting(key []byte) (*pb.PostingList, error) { // Filter and remove STAR_ALL and OP_DELETE Postings idx := 0 for _, postings := range pl.Postings { - if hasDeleteAll(postings) { - return nil, nil - } if postings.Op != Del { pl.Postings[idx] = postings idx++ @@ -412,7 +544,7 @@ func (lc *LocalCache) UpdateDeltasAndDiscardLists() { for key, pl := range lc.plists { data := pl.getMutation(lc.startTs) if len(data) > 0 { - lc.deltas[key] = data + lc.deltas.AddToDeltas(key, data) } lc.maxVersions[key] = pl.maxVersion() // We can't run pl.release() here because LocalCache is still being used by other callers @@ -425,16 +557,19 @@ func (lc *LocalCache) UpdateDeltasAndDiscardLists() { func (lc *LocalCache) fillPreds(ctx *api.TxnContext, gid uint32) { lc.RLock() defer lc.RUnlock() - for key := range lc.deltas { + if err := lc.deltas.IterateKeys(func(key string) error { pk, err := x.Parse([]byte(key)) x.Check(err) if len(pk.Attr) == 0 { - continue + return nil } // Also send the group id that the predicate was being served by. This is useful when // checking if Zero should allow a commit during a predicate move. predKey := fmt.Sprintf("%d-%s", gid, pk.Attr) ctx.Preds = append(ctx.Preds, predKey) + return nil + }); err != nil { + x.Check(err) } ctx.Preds = x.Unique(ctx.Preds) } diff --git a/posting/mvcc.go b/posting/mvcc.go index aaea3f55626..66cec986f7f 100644 --- a/posting/mvcc.go +++ b/posting/mvcc.go @@ -137,9 +137,20 @@ func (ir *incrRollupi) rollUpKey(writer *TxnWriter, key []byte) error { // TODO: When the opRollup is not running the keys from keysPool of ir are dropped. Figure out some // way to handle that. func (ir *incrRollupi) addKeyToBatch(key []byte, priority int) { + // Defensive copy. Callers (notably ReadPostingList's defer and the + // per-predicate mutation pipeline's ProcessCount loop) reuse a single + // key buffer across iterations and mutate its last 8 bytes per uid; + // keeping a slice header that aliases that buffer means every queued + // rollup target collapses to whatever uid was processed last. The + // async rollup then writes a BitCompletePosting to the wrong key, + // overwriting an unrelated posting list with WithDiscard. Take a copy + // here so the rollup queue owns its bytes. + keyCopy := make([]byte, len(key)) + copy(keyCopy, key) + rki := ir.priorityKeys[priority] batch := rki.keysPool.Get().(*[][]byte) - *batch = append(*batch, key) + *batch = append(*batch, keyCopy) if len(*batch) < 16 { rki.keysPool.Put(batch) return @@ -273,8 +284,11 @@ func (txn *Txn) CommitToDisk(writer *TxnWriter, commitTs uint64) error { defer cache.Unlock() var keys []string - for key := range cache.deltas { + if err := cache.deltas.IterateKeys(func(key string) error { keys = append(keys, key) + return nil + }); err != nil { + return err } defer func() { @@ -293,8 +307,15 @@ func (txn *Txn) CommitToDisk(writer *TxnWriter, commitTs uint64) error { err := writer.update(commitTs, func(btxn *badger.Txn) error { for ; idx < len(keys); idx++ { key := keys[idx] - data := cache.deltas[key] - if len(data) == 0 { + data, ok := cache.deltas.GetBytes(key) + if !ok || data == nil { + continue + } + pl := &pb.PostingList{} + if err := proto.Unmarshal(data, pl); err != nil { + return err + } + if len(pl.Postings) == 0 { continue } if ts := cache.maxVersions[key]; ts >= commitTs { @@ -579,7 +600,7 @@ func (ml *MemoryLayer) wait() { ml.cache.wait() } -func (ml *MemoryLayer) updateItemInCache(key string, delta []byte, startTs, commitTs uint64) { +func (ml *MemoryLayer) updateItemInCache(key string, delta *pb.PostingList, startTs, commitTs uint64) { if commitTs == 0 { return } @@ -591,24 +612,19 @@ func (ml *MemoryLayer) updateItemInCache(key string, delta []byte, startTs, comm } val, ok := ml.cache.get([]byte(key)) - if !ok { - return - } - - val.lastUpdate = commitTs - if val.list != nil { - p := new(pb.PostingList) - x.Check(proto.Unmarshal(delta, p)) + if ok && val.list != nil && val.list.minTs <= commitTs { + val.lastUpdate = commitTs - if p.Pack == nil { - val.list.setMutationAfterCommit(startTs, commitTs, p, true) - checkForRollup([]byte(key), val.list) - } else { - // Data was rolled up. TODO figure out how is UpdateCachedKeys getting delta which is pack) - ml.del([]byte(key)) + if val.list != nil { + if delta.Pack == nil { + val.list.setMutationAfterCommit(startTs, commitTs, delta, true) + checkForRollup([]byte(key), val.list) + } else { + // Data was rolled up. TODO figure out how is UpdateCachedKeys getting delta which is pack) + ml.del([]byte(key)) + } } - } } @@ -619,8 +635,11 @@ func (txn *Txn) UpdateCachedKeys(commitTs uint64) { } MemLayerInstance.wait() - for key, delta := range txn.cache.deltas { - MemLayerInstance.updateItemInCache(key, delta, txn.StartTs, commitTs) + if err := txn.cache.deltas.IteratePostings(func(key string, value *pb.PostingList) error { + MemLayerInstance.updateItemInCache(key, value, txn.StartTs, commitTs) + return nil + }); err != nil { + glog.Errorf("UpdateCachedKeys: error while iterating deltas: %v", err) } } @@ -674,7 +693,16 @@ func ReadPostingList(key []byte, it *badger.Iterator) (*List, error) { } l := new(List) - l.key = key + // Copy the key bytes. The caller (mutation pipeline ProcessCount and + // similar paths) reuses a single key buffer across loop iterations and + // mutates its trailing 8 bytes to scan many uids; without copying here + // the list's l.key field aliases that buffer, so by the time the list + // is read back from the in-memory cache (saveInCache stashes a + // copyList that shares the key alias) the bytes have changed. The + // async rollup path then takes alloc.Copy(l.key) to build the rolled- + // up KV's key and ends up writing a BitCompletePosting (with + // WithDiscard) to a different list's key — corrupting that list. + l.key = append([]byte(nil), key...) l.plist = new(pb.PostingList) l.mutationMap = newMutableLayer() l.minTs = 0 diff --git a/posting/mvcc_test.go b/posting/mvcc_test.go index 1fab24d9520..8588a92d2c8 100644 --- a/posting/mvcc_test.go +++ b/posting/mvcc_test.go @@ -74,7 +74,7 @@ func TestCacheAfterDeltaUpdateRecieved(t *testing.T) { // Write delta to disk and call update txn := Oracle().RegisterStartTs(5) - txn.cache.deltas[string(key)] = delta + txn.cache.deltas.AddToDeltas(string(key), delta) writer := NewTxnWriter(pstore) require.NoError(t, txn.CommitToDisk(writer, 15)) @@ -146,6 +146,8 @@ func BenchmarkTestCache(b *testing.B) { } func TestRollupTimestamp(t *testing.T) { + require.NoError(t, schema.ParseBytes([]byte("rollup: [uid] ."), 1)) + attr := x.AttrInRootNamespace("rollup") key := x.DataKey(attr, 1) // 3 Delta commits. @@ -213,7 +215,7 @@ func TestCacheStaleWhenMaxTsLessThanReadTs(t *testing.T) { require.NoError(t, err) txn1 := Oracle().RegisterStartTs(5) - txn1.cache.deltas[string(key)] = delta1 + txn1.cache.deltas.AddToDeltas(string(key), delta1) writer1 := NewTxnWriter(pstore) require.NoError(t, txn1.CommitToDisk(writer1, 10)) @@ -246,7 +248,7 @@ func TestCacheStaleWhenMaxTsLessThanReadTs(t *testing.T) { require.NoError(t, err) txn2 := Oracle().RegisterStartTs(15) - txn2.cache.deltas[string(key)] = delta2 + txn2.cache.deltas.AddToDeltas(string(key), delta2) writer2 := NewTxnWriter(pstore) require.NoError(t, txn2.CommitToDisk(writer2, 20)) @@ -604,6 +606,8 @@ func TestResetIfCurrentLeavesADroppedEntryOut(t *testing.T) { } func TestPostingListRead(t *testing.T) { + require.NoError(t, schema.ParseBytes([]byte("emptypl: [uid] ."), 1)) + attr := x.AttrInRootNamespace("emptypl") key := x.DataKey(attr, 1) diff --git a/posting/oracle.go b/posting/oracle.go index 362d00667d7..0454dc960a7 100644 --- a/posting/oracle.go +++ b/posting/oracle.go @@ -9,6 +9,7 @@ import ( "context" "encoding/hex" "math" + "sort" "sync" "sync/atomic" "time" @@ -16,6 +17,7 @@ import ( "github.com/dgraph-io/badger/v4" "github.com/golang/glog" ostats "go.opencensus.io/stats" + "google.golang.org/protobuf/proto" "github.com/dgraph-io/dgraph/v25/protos/pb" "github.com/dgraph-io/dgraph/v25/tok/index" @@ -54,6 +56,84 @@ type Txn struct { lastUpdate time.Time cache *LocalCache // This pointer does not get modified. + + pointers [](*[]byte) +} + +func (txn *Txn) AddPointer(p *[]byte) { + if txn.pointers == nil { + txn.pointers = make([](*[]byte), 1) + txn.pointers[0] = p + } + txn.pointers = append(txn.pointers, p) +} + +func (txn *Txn) GetPointers() [](*[]byte) { + return txn.pointers +} + +func SortAndDedupPostings(postings []*pb.Posting) []*pb.Posting { + // Sort postings by UID + sort.Slice(postings, func(i, j int) bool { + return postings[i].Uid < postings[j].Uid + }) + + //In-place filtering: keep only the last occurrence for each UID + n := 0 // write index + for i := 0; i < len(postings); { + j := i + 1 + // Skip all postings with same UID + for j < len(postings) && postings[j].Uid == postings[i].Uid { + j++ + } + // Keep only the last posting for this UID + postings[n] = postings[j-1] + n++ + i = j + } + return postings[:n] +} + +func (txn *Txn) AddDelta(key string, input *pb.PostingList, doSortAndDedup bool, addToList bool) (*pb.PostingList, error) { + txn.cache.Lock() + defer txn.cache.Unlock() + + pl := new(pb.PostingList) + + if addToList { + prevDelta, ok := txn.cache.deltas.Get(key) + if ok { + pl.Postings = append(pl.Postings, prevDelta.Postings...) + } + } + + pl.Postings = append(pl.Postings, input.Postings...) + + if doSortAndDedup { + pl.Postings = SortAndDedupPostings(pl.Postings) + } + + newPl, err := proto.Marshal(pl) + if err != nil { + glog.Errorf("Error marshalling posting list: %v", err) + return nil, err + } + + txn.cache.deltas.AddToDeltas(key, newPl) + + list, listOk := txn.cache.plists[key] + if listOk { + list.setMutation(txn.StartTs, newPl) + } + return pl, nil +} + +func (txn *Txn) LockCache() { + txn.cache.Lock() +} + +func (txn *Txn) UnlockCache() { + txn.cache.Unlock() } // struct to implement Txn interface from vector-indexer @@ -324,8 +404,11 @@ func (o *oracle) ProcessDelta(delta *pb.OracleDelta) { for _, status := range delta.Txns { txn := o.pendingTxns[status.StartTs] if txn != nil && status.CommitTs > 0 { - for k := range txn.cache.deltas { - IncrRollup.addKeyToBatch([]byte(k), 0) + if err := txn.cache.deltas.IterateBytes(func(key string, value []byte) error { + IncrRollup.addKeyToBatch([]byte(key), 0) + return nil + }); err != nil { + glog.Errorf("ProcessDelta: error while iterating deltas for txn %d: %v", status.StartTs, err) } } delete(o.pendingTxns, status.StartTs) @@ -379,17 +462,6 @@ func (o *oracle) GetTxn(startTs uint64) *Txn { return o.pendingTxns[startTs] } -func (txn *Txn) matchesDelta(ok func(key []byte) bool) bool { - txn.Lock() - defer txn.Unlock() - for key := range txn.cache.deltas { - if ok([]byte(key)) { - return true - } - } - return false -} - // IterateTxns returns a list of start timestamps for currently pending transactions, which match // the provided function. func (o *oracle) IterateTxns(ok func(key []byte) bool) []uint64 { @@ -397,8 +469,13 @@ func (o *oracle) IterateTxns(ok func(key []byte) bool) []uint64 { defer o.RUnlock() var timestamps []uint64 for startTs, txn := range o.pendingTxns { - if txn.matchesDelta(ok) { - timestamps = append(timestamps, startTs) + if err := txn.cache.deltas.IterateBytes(func(key string, value []byte) error { + if ok([]byte(key)) { + timestamps = append(timestamps, startTs) + } + return nil + }); err != nil { + glog.Errorf("IterateTxns: error while iterating deltas for txn %d: %v", startTs, err) } } return timestamps diff --git a/posting/pipeline_test.go b/posting/pipeline_test.go new file mode 100644 index 00000000000..2d218e13daa --- /dev/null +++ b/posting/pipeline_test.go @@ -0,0 +1,1557 @@ +/* + * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package posting + +// Tests for MutationPipeline correctness — focused on correctness gaps vs the +// legacy runMutation path discovered during code review. + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/dgraph-io/dgraph/v25/protos/pb" + "github.com/dgraph-io/dgraph/v25/schema" + "github.com/dgraph-io/dgraph/v25/types" + "github.com/dgraph-io/dgraph/v25/x" +) + +// commitPipelineTxn flushes a pipeline txn to Badger and marks it committed. +// Mirrors what draft.go does after applyMutations returns. +func commitPipelineTxn(t *testing.T, txn *Txn, commitTs uint64) { + t.Helper() + txn.Update() + txn.UpdateCachedKeys(commitTs) + writer := NewTxnWriter(pstore) + require.NoError(t, txn.CommitToDisk(writer, commitTs)) + require.NoError(t, writer.Flush()) +} + +// applyEdges runs edges through a fresh pipeline txn and commits them. +func applyEdges(t *testing.T, startTs, commitTs uint64, edges ...*pb.DirectedEdge) { + t.Helper() + txn := Oracle().RegisterStartTs(startTs) + mp := NewMutationPipeline(txn) + require.NoError(t, mp.Process(context.Background(), edges)) + commitPipelineTxn(t, txn, commitTs) +} + +// applyEdgesLegacy runs edges through the legacy runMutation path and commits them. +// Used to verify that legacy and pipeline produce the same results. +func applyEdgesLegacy(t *testing.T, startTs, commitTs uint64, edges ...*pb.DirectedEdge) { + t.Helper() + txn := Oracle().RegisterStartTs(startTs) + for _, edge := range edges { + require.NoError(t, runMutation(context.Background(), edge, txn)) + } + commitPipelineTxn(t, txn, commitTs) +} + +// forwardUids returns the target uids stored in a forward data list at readTs. +func forwardUids(t *testing.T, attr string, entity, readTs uint64) []uint64 { + t.Helper() + l, err := GetNoStore(x.DataKey(attr, entity), readTs) + require.NoError(t, err) + return uids(l, readTs) +} + +// reverseUids returns the source uids stored in a reverse index list at readTs. +func reverseUids(t *testing.T, attr string, target, readTs uint64) []uint64 { + t.Helper() + l, err := GetNoStore(x.ReverseKey(attr, target), readTs) + require.NoError(t, err) + return uids(l, readTs) +} + +// indexUidsForVal reads the exact-index entries for a plain (non-lang) string +// @index predicate. It reuses indexUidsForLang with an empty language tag. +func indexUidsForVal(t *testing.T, attr, val string, readTs uint64) []uint64 { + t.Helper() + return indexUidsForLang(t, attr, "", val, readTs) +} + +// Each test uses a unique timestamp band so the Oracle's maxAssignedTs from one +// test never interferes with GetScalarList reads in a subsequent test. +// Band layout (startTs / commitTs / readTs): +// Pipeline tests: 100-199, 200-299, 300-399, 400-499, 900-999 +// Legacy tests: 500-599, 600-699, 700-799, 800-899, 1000-1099 +// Lang index: Pipeline 900-999, Legacy 1000-1099 +// Star-delete+SET: Pipeline 1100-1199, Legacy 1200-1299 +// Single SET+DEL: Pipeline 1300-1399, Legacy 1400-1499 +// DEL wrong target: Pipeline 1500-1599, Legacy 1600-1699 +// Lang SET-then-DEL: Pipeline 1700-1799, Legacy 1800-1899 +// Multi-SET scalar: Pipeline 1900-1999, Legacy 2000-2099 +// Case-insensitive dup DEL: Pipeline 2100-2199, Legacy 2200-2299 +// Channel deadlock: 2300-2399 +// Non-matching DEL no-op: Pipeline 2400-2499, Legacy 2500-2599 + +// TestMutationPipelineSingleUidReverseUpdate is the regression test for the +// handleOldDeleteForSingle value-comparison bug on non-list uid @reverse predicates. +// +// Root cause: handleOldDeleteForSingle compares posting.Value (always nil for uid +// postings — the target uid lives in posting.Uid) using string byte comparison. +// `string(nil) == string(nil)` is always true, so the function concludes "new value +// equals old value → no-op" for every uid SET, clears the accumulated postings to an +// empty PostingList, and neither the forward write nor the reverse-edge update ever +// happens. +// +// Expected (correct) behaviour after the fix: +// - alice's forward list is updated to bob. +// - The new reverse edge bob ← alice is created. +// - The stale reverse edge charlie ← alice is removed. +// +// Observed (buggy) behaviour without the fix: +// - alice's forward list still points to charlie (SET is a no-op). +// - bob's reverse list remains empty. +// - charlie's reverse list still contains alice. +func TestMutationPipelineSingleUidReverseUpdate(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + ) + + // Step 1 — initial state: alice → best_friend → charlie. + applyEdges(t, 110, 111, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + // Sanity-check initial state. + require.Equal(t, []uint64{charlie}, forwardUids(t, attr, alice, 112), + "initial: alice should point to charlie") + require.Equal(t, []uint64{alice}, reverseUids(t, attr, charlie, 112), + "initial: charlie's reverse should contain alice") + require.Empty(t, reverseUids(t, attr, bob, 112), + "initial: bob's reverse should be empty") + + // Step 2 — update: SET alice → best_friend → bob (replaces charlie). + applyEdges(t, 120, 121, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(122) + + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, readTs), + "alice's best_friend must be updated to bob") + require.Contains(t, reverseUids(t, attr, bob, readTs), alice, + "bob's reverse index must now contain alice") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "charlie's reverse index must no longer contain alice") +} + +// TestMutationPipelineSingleUidReverseUpdateChain tests three consecutive updates to +// the same non-list uid @reverse predicate to verify that each transition is applied +// correctly and no stale reverse edges accumulate. +// +// alice → charlie → alice → bob → alice → dave +func TestMutationPipelineSingleUidReverseUpdateChain(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + dave = uint64(40) + ) + + // alice → charlie + applyEdges(t, 210, 211, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + // alice → bob (first replacement) + applyEdges(t, 220, 221, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET, + }) + + const afterBob = uint64(222) + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, afterBob)) + require.Contains(t, reverseUids(t, attr, bob, afterBob), alice) + require.NotContains(t, reverseUids(t, attr, charlie, afterBob), alice) + + // alice → dave (second replacement) + applyEdges(t, 230, 231, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: dave, Op: pb.DirectedEdge_SET, + }) + + const afterDave = uint64(232) + require.Equal(t, []uint64{dave}, forwardUids(t, attr, alice, afterDave), + "alice's best_friend must be dave after second replacement") + require.Contains(t, reverseUids(t, attr, dave, afterDave), alice, + "dave's reverse must contain alice") + require.NotContains(t, reverseUids(t, attr, bob, afterDave), alice, + "bob's reverse must no longer contain alice after second replacement") + require.NotContains(t, reverseUids(t, attr, charlie, afterDave), alice, + "charlie's reverse must still not contain alice") +} + +// TestMutationPipelineSingleUidReverseSetSameValue tests that re-setting the same +// uid value is correctly treated as a no-op: the forward list is unchanged and no +// spurious reverse-edge churn occurs. +func TestMutationPipelineSingleUidReverseSetSameValue(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + charlie = uint64(30) + ) + + // Initial: alice → charlie. + applyEdges(t, 310, 311, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + // Re-set the same value. + applyEdges(t, 320, 321, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(322) + + require.Equal(t, []uint64{charlie}, forwardUids(t, attr, alice, readTs), + "alice's best_friend must still be charlie after no-op re-set") + require.Contains(t, reverseUids(t, attr, charlie, readTs), alice, + "charlie's reverse must still contain alice after no-op re-set") +} + +// TestMutationPipelineSingleUidReverseDelete tests that DEL on a non-list uid +// @reverse predicate removes both the forward entry and the reverse index entry. +func TestMutationPipelineSingleUidReverseDelete(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + charlie = uint64(30) + ) + + // Initial: alice → charlie. + applyEdges(t, 410, 411, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + // Delete alice → charlie. For uid predicates, ValueId carries the target uid. + applyEdges(t, 420, 421, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_DEL, + }) + + const readTs = uint64(422) + + require.Empty(t, forwardUids(t, attr, alice, readTs), + "alice's best_friend must be empty after DEL") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "charlie's reverse must not contain alice after DEL") +} + +// --------------------------------------------------------------------------- +// Legacy-path mirrors — same scenarios as above but via runMutation. +// These establish a baseline: they must all pass both before and after the +// pipeline fix. Any divergence between Legacy* and non-Legacy* results +// points to a pipeline-specific regression. +// --------------------------------------------------------------------------- + +func TestLegacySingleUidReverseUpdate(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + ) + + applyEdgesLegacy(t, 510, 511, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + applyEdgesLegacy(t, 520, 521, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(522) + + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, readTs), + "legacy: alice's best_friend must be updated to bob") + require.Contains(t, reverseUids(t, attr, bob, readTs), alice, + "legacy: bob's reverse index must now contain alice") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "legacy: charlie's reverse index must no longer contain alice") +} + +func TestLegacySingleUidReverseUpdateChain(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + dave = uint64(40) + ) + + applyEdgesLegacy(t, 610, 611, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + applyEdgesLegacy(t, 620, 621, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET, + }) + + const afterBob = uint64(622) + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, afterBob)) + require.Contains(t, reverseUids(t, attr, bob, afterBob), alice) + require.NotContains(t, reverseUids(t, attr, charlie, afterBob), alice) + + applyEdgesLegacy(t, 630, 631, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: dave, Op: pb.DirectedEdge_SET, + }) + + const afterDave = uint64(632) + require.Equal(t, []uint64{dave}, forwardUids(t, attr, alice, afterDave), + "legacy: alice's best_friend must be dave after second replacement") + require.Contains(t, reverseUids(t, attr, dave, afterDave), alice, + "legacy: dave's reverse must contain alice") + require.NotContains(t, reverseUids(t, attr, bob, afterDave), alice, + "legacy: bob's reverse must no longer contain alice") + require.NotContains(t, reverseUids(t, attr, charlie, afterDave), alice, + "legacy: charlie's reverse must still not contain alice") +} + +func TestLegacySingleUidReverseSetSameValue(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + charlie = uint64(30) + ) + + applyEdgesLegacy(t, 710, 711, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + applyEdgesLegacy(t, 720, 721, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(722) + + require.Equal(t, []uint64{charlie}, forwardUids(t, attr, alice, readTs), + "legacy: alice's best_friend must still be charlie after no-op re-set") + require.Contains(t, reverseUids(t, attr, charlie, readTs), alice, + "legacy: charlie's reverse must still contain alice after no-op re-set") +} + +func TestLegacySingleUidReverseDelete(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + charlie = uint64(30) + ) + + applyEdgesLegacy(t, 810, 811, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + applyEdgesLegacy(t, 820, 821, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_DEL, + }) + + const readTs = uint64(822) + + require.Empty(t, forwardUids(t, attr, alice, readTs), + "legacy: alice's best_friend must be empty after DEL") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "legacy: charlie's reverse must not contain alice after DEL") +} + +// --------------------------------------------------------------------------- +// Language predicate @index stale entry tests +// --------------------------------------------------------------------------- + +// indexUidsForLang returns entity UIDs found in the exact index for the given +// language-tagged value. attr must already be namespace-qualified. +func indexUidsForLang(t *testing.T, attr, lang, val string, readTs uint64) []uint64 { + t.Helper() + tokens, err := indexTokens(context.Background(), &indexMutationInfo{ + tokenizers: schema.State().Tokenizer(context.Background(), attr), + edge: &pb.DirectedEdge{Attr: attr, Lang: lang}, + val: types.Val{Tid: types.StringID, Value: []byte(val)}, + }) + require.NoError(t, err) + var result []uint64 + for _, token := range tokens { + l, err := GetNoStore(x.IndexKey(attr, token), readTs) + require.NoError(t, err) + result = append(result, uids(l, readTs)...) + } + return result +} + +// TestMutationPipelineLangIndexStaleEntry is the regression test for the +// ProcessList language-predicate stale index-entry bug. +// +// Root cause: ProcessList accumulates {SET "Alicia"} and passes it directly to +// InsertTokenizerIndexes, which emits only the SET index entry for the new +// value. It never reads the committed data key, so it never generates a DEL +// index entry for the old "Alice" — that entry persists in the index. +// +// The legacy path (AddMutationWithIndex → addMutationHelper) correctly calls +// findPosting to retrieve the old value then emits both: +// +// addIndexMutations(DEL, "Alice") and addIndexMutations(SET, "Alicia") +// +// Expected (correct) behaviour: +// - index("Alicia") contains alice. +// - index("Alice") does NOT contain alice. +// +// Observed (buggy) behaviour without the fix: +// - index("Alice") still contains alice (stale entry). +func TestMutationPipelineLangIndexStaleEntry(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @lang @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + // Step 1 — initial state: alice's name@en = "Alice". + applyEdges(t, 910, 911, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForLang(t, attr, "en", "Alice", 912), alice, + "initial: index for 'Alice' must contain alice") + require.NotContains(t, indexUidsForLang(t, attr, "en", "Alicia", 912), alice, + "initial: index for 'Alicia' must not contain alice") + + // Step 2 — update: SET alice's name@en = "Alicia" (replaces "Alice"). + applyEdges(t, 920, 921, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alicia"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(922) + + require.Contains(t, indexUidsForLang(t, attr, "en", "Alicia", readTs), alice, + "index for 'Alicia' must contain alice after update") + require.NotContains(t, indexUidsForLang(t, attr, "en", "Alice", readTs), alice, + "index for 'Alice' must not contain alice after update — stale entry bug") +} + +// TestLegacyLangIndexStaleEntry mirrors TestMutationPipelineLangIndexStaleEntry +// via the legacy runMutation path to establish that the legacy path is correct +// and the stale entry issue is pipeline-specific. +func TestLegacyLangIndexStaleEntry(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @lang @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + applyEdgesLegacy(t, 1010, 1011, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }) + applyEdgesLegacy(t, 1020, 1021, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alicia"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(1022) + + require.Contains(t, indexUidsForLang(t, attr, "en", "Alicia", readTs), alice, + "legacy: index for 'Alicia' must contain alice after update") + require.NotContains(t, indexUidsForLang(t, attr, "en", "Alice", readTs), alice, + "legacy: index for 'Alice' must not contain alice after update") +} + +// --------------------------------------------------------------------------- +// Star-delete + same-batch SET tests (Bug 4) +// --------------------------------------------------------------------------- + +// TestMutationPipelineStarDeleteThenSet is the regression test for the +// deleteAll-marker overwrite bug in MutationPipeline. +// +// Root cause: Process() handles a star-delete by calling l.handleDeleteAll on +// the List object in cache.plists, which sets deleteAllMarker and +// currentEntries=[{DeleteAll}]. Subsequent ProcessList processing for the same +// key calls AddDelta, which calls list.setCurrentEntries — unconditionally +// resetting deleteAllMarker to math.MaxUint64 and discarding the deleteAll. +// The committed friends (bob, charlie) are therefore NOT cleared. +// +// Expected (correct) behaviour: +// - alice's friends = [dave] only. +// - bob's and charlie's reverse edges are gone. +// - dave's reverse edge is present. +// +// Observed (buggy) behaviour without the fix: +// - alice's friends = [bob, charlie, dave] (old friends not deleted). +func TestMutationPipelineStarDeleteThenSet(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`friend: [uid] @reverse .`), 1)) + + attr := x.AttrInRootNamespace("friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + dave = uint64(40) + ) + + // Step 1 — initial state: alice has friends bob and charlie. + applyEdges(t, 1110, 1111, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET}, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET}, + ) + + require.Len(t, forwardUids(t, attr, alice, 1112), 2, + "initial: alice should have two friends") + + // Step 2 — batch: star-delete alice's friends, then add dave. + applyEdges(t, 1120, 1121, + &pb.DirectedEdge{Entity: alice, Attr: attr, Value: []byte(x.Star), Op: pb.DirectedEdge_DEL}, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: dave, Op: pb.DirectedEdge_SET}, + ) + + const readTs = uint64(1122) + require.Equal(t, []uint64{dave}, forwardUids(t, attr, alice, readTs), + "alice's friends must be only dave after star-delete + set") + require.Contains(t, reverseUids(t, attr, dave, readTs), alice, + "dave's reverse must contain alice") + require.NotContains(t, reverseUids(t, attr, bob, readTs), alice, + "bob's reverse must not contain alice after star-delete") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "charlie's reverse must not contain alice after star-delete") +} + +// TestLegacyStarDeleteThenSet mirrors TestMutationPipelineStarDeleteThenSet via +// the legacy runMutation path to establish that the legacy path is correct. +func TestLegacyStarDeleteThenSet(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`friend: [uid] @reverse .`), 1)) + + attr := x.AttrInRootNamespace("friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + dave = uint64(40) + ) + + applyEdgesLegacy(t, 1210, 1211, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET}, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET}, + ) + + applyEdgesLegacy(t, 1220, 1221, + &pb.DirectedEdge{Entity: alice, Attr: attr, Value: []byte(x.Star), Op: pb.DirectedEdge_DEL}, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: dave, Op: pb.DirectedEdge_SET}, + ) + + const readTs = uint64(1222) + + require.Equal(t, []uint64{dave}, forwardUids(t, attr, alice, readTs), + "legacy: alice's friends must be only dave after star-delete + set") + require.Contains(t, reverseUids(t, attr, dave, readTs), alice, + "legacy: dave's reverse must contain alice") + require.NotContains(t, reverseUids(t, attr, bob, readTs), alice, + "legacy: bob's reverse must not contain alice after star-delete") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "legacy: charlie's reverse must not contain alice after star-delete") +} + +// --------------------------------------------------------------------------- +// ProcessSingle: SET + DEL different targets in same batch (Bug 5) +// --------------------------------------------------------------------------- + +// TestMutationPipelineSingleUidSetAndDelDifferentTargets is the regression test +// for the nil==nil value-comparison bug in ProcessSingle's exists=true DEL branch. +// +// Root cause: when a SET alice→bob is already accumulated for alice, a subsequent +// DEL alice→charlie in the same batch enters the `exists=true, edge.Op==DEL` branch. +// The check `string(edge.Value) == string(oldVal.Value)` compares nil==nil (both +// Value fields are nil for uid postings — the uid lives in posting.Uid), which is +// always true. setPosting() therefore REPLACES the accumulated {SET bob} with +// {DEL charlie}. The SET is lost. +// +// Expected (correct) behaviour after the fix: +// - alice's best_friend is bob. +// - bob's reverse index contains alice. +// - charlie's reverse index does NOT contain alice. +// +// Observed (buggy) behaviour without the fix: +// - alice has no best_friend (the SET bob was replaced by DEL charlie). +// - bob's reverse index is empty. +func TestMutationPipelineSingleUidSetAndDelDifferentTargets(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + ) + + // Step 1 — initial state: alice → best_friend → charlie. + applyEdges(t, 1310, 1311, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + // Step 2 — batch: SET alice→bob first, then DEL alice→charlie. + // The SET and DEL target different uids (bob ≠ charlie), triggering Bug 5. + applyEdges(t, 1320, 1321, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET}, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_DEL}, + ) + + const readTs = uint64(1322) + + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, readTs), + "alice's best_friend must be bob after set-bob + del-charlie batch") + require.Contains(t, reverseUids(t, attr, bob, readTs), alice, + "bob's reverse must contain alice") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "charlie's reverse must not contain alice after del") +} + +// TestLegacySingleUidSetAndDelDifferentTargets mirrors +// TestMutationPipelineSingleUidSetAndDelDifferentTargets via the legacy path. +func TestLegacySingleUidSetAndDelDifferentTargets(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + ) + + applyEdgesLegacy(t, 1410, 1411, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_SET, + }) + + applyEdgesLegacy(t, 1420, 1421, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET}, + &pb.DirectedEdge{Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_DEL}, + ) + + const readTs = uint64(1422) + + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, readTs), + "legacy: alice's best_friend must be bob after set-bob + del-charlie batch") + require.Contains(t, reverseUids(t, attr, bob, readTs), alice, + "legacy: bob's reverse must contain alice") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "legacy: charlie's reverse must not contain alice after del") +} + +// --------------------------------------------------------------------------- +// ProcessSingle: DEL wrong target uid (Bug 6) +// --------------------------------------------------------------------------- + +// TestMutationPipelineSingleUidDelWrongTarget documents the expected behaviour +// when a DEL targets a uid that does NOT match the committed value. +// +// Root cause (Bug 6): in ProcessSingle's exists=false DEL branch, the comparison +// `string(oldVal.Value) == string(edge.Value)` is nil==nil for uid postings, so +// setPosting() is always called regardless of whether the DEL target uid +// (edge.ValueId) matches the committed uid (oldVal.Uid). This creates a spurious +// {DEL wrong_uid} delta. The delta has no visible effect on forward data or the +// reverse index (because the wrong uid was never stored for that entity), so the +// observable state is identical to the correct no-op. +// +// Both pipeline and legacy should pass: alice retains bob, bob's reverse keeps +// alice, and charlie's reverse (which never contained alice) is unchanged. +func TestMutationPipelineSingleUidDelWrongTarget(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + ) + + // Step 1 — initial state: alice → best_friend → bob. + applyEdges(t, 1510, 1511, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET, + }) + + // Step 2 — DEL alice→charlie (charlie is the wrong target; alice has bob). + // This DEL is a no-op: alice does not have charlie as best_friend. + applyEdges(t, 1520, 1521, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_DEL, + }) + + const readTs = uint64(1522) + + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, readTs), + "alice's best_friend must still be bob — DEL wrong target is a no-op") + require.Contains(t, reverseUids(t, attr, bob, readTs), alice, + "bob's reverse must still contain alice") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "charlie's reverse must not contain alice — no spurious reverse entry") +} + +// TestLegacySingleUidDelWrongTarget mirrors TestMutationPipelineSingleUidDelWrongTarget +// via the legacy path to confirm both paths treat DEL-wrong-target as a no-op. +func TestLegacySingleUidDelWrongTarget(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`best_friend: uid @reverse .`), 1)) + + attr := x.AttrInRootNamespace("best_friend") + const ( + alice = uint64(10) + bob = uint64(20) + charlie = uint64(30) + ) + + applyEdgesLegacy(t, 1610, 1611, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: bob, Op: pb.DirectedEdge_SET, + }) + + applyEdgesLegacy(t, 1620, 1621, &pb.DirectedEdge{ + Entity: alice, Attr: attr, ValueId: charlie, Op: pb.DirectedEdge_DEL, + }) + + const readTs = uint64(1622) + + require.Equal(t, []uint64{bob}, forwardUids(t, attr, alice, readTs), + "legacy: alice's best_friend must still be bob — DEL wrong target is a no-op") + require.Contains(t, reverseUids(t, attr, bob, readTs), alice, + "legacy: bob's reverse must still contain alice") + require.NotContains(t, reverseUids(t, attr, charlie, readTs), alice, + "legacy: charlie's reverse must not contain alice") +} + +// --------------------------------------------------------------------------- +// ProcessList lang: SET new then DEL old in same batch (Scenario 4) +// --------------------------------------------------------------------------- + +// TestMutationPipelineLangSetNewThenDelOldSameBatch is the regression test for +// the index-loss bug when a lang predicate batch contains SET new-value followed +// by DEL old-value for the same language tag. +// +// Root cause: ProcessList accumulates postings in a MutableLayer keyed by +// FingerprintEdge(lang). Both SET "Alicia"@en and DEL "Alice"@en share the +// same fingerprint (fingerprint of "en"). insertPosting stores them under the +// same uid slot and the second write (DEL "Alice") overwrites the first (SET +// "Alicia"). InsertTokenizerIndexes then only sees {DEL "Alice"} — it emits the +// removal of the old index entry but never emits the SET entry for the new value. +// +// The legacy path (addMutationHelper per edge) generates index mutations at +// edge-processing time, BEFORE the same-uid overwrite affects the mutation layer. +// So legacy produces the correct index even though the forward scalar value is +// also corrupted by the overwrite (no value stored for alice's name@en — a +// separate bug shared by both paths). +// +// Expected (correct) behaviour: +// - index("Alicia") contains alice. +// - index("Alice") does NOT contain alice. +// +// Observed (buggy) behaviour without the fix: +// - index("Alicia") does NOT contain alice (SET entry was lost). +// - index("Alice") also no longer contains alice (DEL was applied correctly). +func TestMutationPipelineLangSetNewThenDelOldSameBatch(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @lang @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + // Step 1 — initial state: alice's name@en = "Alice". + applyEdges(t, 1710, 1711, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForLang(t, attr, "en", "Alice", 1712), alice, + "initial: index for 'Alice' must contain alice") + + // Step 2 — batch: SET "Alicia"@en first, then DEL "Alice"@en. + // Both share fingerprint("en") → DEL overwrites SET in the MutableLayer. + applyEdges(t, 1720, 1721, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alicia"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_DEL, + }, + ) + + const readTs = uint64(1722) + + require.Contains(t, indexUidsForLang(t, attr, "en", "Alicia", readTs), alice, + "index for 'Alicia' must contain alice after set-new + del-old batch") + require.NotContains(t, indexUidsForLang(t, attr, "en", "Alice", readTs), alice, + "index for 'Alice' must not contain alice after del-old") +} + +// TestLegacyLangSetNewThenDelOldSameBatch mirrors Scenario 4 via the legacy path. +// Legacy generates index mutations at edge-processing time, so the SET "Alicia" +// index entry is created before the same-uid overwrite discards the SET posting. +func TestLegacyLangSetNewThenDelOldSameBatch(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @lang @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + applyEdgesLegacy(t, 1810, 1811, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }) + + applyEdgesLegacy(t, 1820, 1821, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alicia"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_SET, + }, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Lang: "en", Op: pb.DirectedEdge_DEL, + }, + ) + + const readTs = uint64(1822) + + require.Contains(t, indexUidsForLang(t, attr, "en", "Alicia", readTs), alice, + "legacy: index for 'Alicia' must contain alice after set-new + del-old batch") + require.NotContains(t, indexUidsForLang(t, attr, "en", "Alice", readTs), alice, + "legacy: index for 'Alice' must not contain alice after del-old") +} + +// --------------------------------------------------------------------------- +// Multiple SETs same uid same batch — scalar @index (Scenario 5) +// --------------------------------------------------------------------------- + +// TestMutationPipelineMultipleSetsSameUidScalarIndex verifies that when a batch +// contains two SET edges for the same entity on a scalar @index predicate, only +// the LAST value ends up in the index (last-SET-wins), and the intermediate +// value never appears in the index. +// +// This scenario is NOT pipeline-specific; both pipeline and legacy apply +// last-SET-wins semantics. The test exists to confirm that no intermediate index +// entry is created for the first SET value. +// +// Setup: alice has nickname = "v1" committed. +// Batch: SET alice nickname = "v1" (same as committed — redundant) +// +// SET alice nickname = "v2" (the desired new value) +// +// Expected: +// - index("v2") contains alice. +// - index("v1") does NOT contain alice (removed by handleOldDeleteForSingle's +// synthetic DEL for the committed value; the intermediate SET v1 was absorbed +// into SET v2 by last-SET-wins accumulation, so no redundant v1 index entry). +func TestMutationPipelineMultipleSetsSameUidScalarIndex(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`nickname: string @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("nickname") + const alice = uint64(10) + + // Step 1 — commit initial value "v1". + applyEdges(t, 1910, 1911, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("v1"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForVal(t, attr, "v1", 1912), alice, + "initial: index('v1') must contain alice") + + // Step 2 — batch: redundant SET v1 + final SET v2. + // ProcessSingle accumulates SET v1 then SET v2 → last-SET-wins → [{SET v2}]. + // handleOldDeleteForSingle reads committed v1, synthesises {DEL v1}. + // InsertTokenizerIndexes: SET v2 (add) + DEL v1 (remove). + applyEdges(t, 1920, 1921, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("v1"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("v2"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }, + ) + + const readTs = uint64(1922) + + require.Contains(t, indexUidsForVal(t, attr, "v2", readTs), alice, + "index('v2') must contain alice after last-SET-wins") + require.NotContains(t, indexUidsForVal(t, attr, "v1", readTs), alice, + "index('v1') must not contain alice — old value removed by synthetic DEL") +} + +// TestLegacyMultipleSetsSameUidScalarIndex mirrors Scenario 5 via the legacy path. +func TestLegacyMultipleSetsSameUidScalarIndex(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`nickname: string @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("nickname") + const alice = uint64(10) + + applyEdgesLegacy(t, 2010, 2011, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("v1"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + applyEdgesLegacy(t, 2020, 2021, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("v1"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("v2"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }, + ) + + const readTs = uint64(2022) + + require.Contains(t, indexUidsForVal(t, attr, "v2", readTs), alice, + "legacy: index('v2') must contain alice after last-SET-wins") + require.NotContains(t, indexUidsForVal(t, attr, "v1", readTs), alice, + "legacy: index('v1') must not contain alice — old value removed") +} + +// --------------------------------------------------------------------------- +// Bug 5: ProcessSingle case-insensitive tokenizer — duplicate synthetic DEL +// --------------------------------------------------------------------------- + +// TestMutationPipelineCaseInsensitiveTokenizerDupDel is the regression test for the +// index-loss bug that occurs when all three of the following hold: +// +// 1. The predicate uses a case-insensitive tokenizer (e.g. "term", which applies a +// lowercase filter so "Apple" and "APPLE" hash to the same bucket "apple"). +// 2. The user explicitly sends a DEL for the old value alongside a SET for a new +// value in the same batch. +// 3. The old and new values produce the same token after normalization. +// +// Root cause: ProcessSingle accumulates {DEL "Apple", SET "APPLE"} from the user +// edges. handleOldDeleteForSingle then reads the committed value "Apple" and sees +// that it differs from the new value "APPLE" (byte-level comparison), so it appends +// a synthetic DEL "Apple". The resulting PostingList has three entries: +// +// [{DEL "Apple"}, {SET "APPLE"}, {DEL "Apple" (synthetic)}] +// +// InsertTokenizerIndexes only applies the isSingleEdge reversal when len == 2. +// With len == 3 no reversal fires, so the postings are fed to insertPosting in +// original order. Both DEL "Apple" entries produce the same index key ("apple") +// and the same entity uid. The three insertPosting calls leave currentEntries[alice] +// as DEL — so alice is removed from the "apple" bucket even though her tag is now +// "APPLE" (which tokenises to "apple"). +// +// Expected (correct) behaviour: +// - index("APPLE") — i.e. the "apple" bucket — still contains alice after the +// batch, because her new value "APPLE" maps to that bucket. +// +// Observed (buggy) behaviour without the fix: +// - index("APPLE") does NOT contain alice — she was incorrectly removed. +func TestMutationPipelineCaseInsensitiveTokenizerDupDel(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`casetag: string @index(term) .`), 1)) + + attr := x.AttrInRootNamespace("casetag") + const alice = uint64(10) + + // Step 1 — commit initial value "Apple". Term-tokenises to "apple". + applyEdges(t, 2110, 2111, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Apple"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForVal(t, attr, "Apple", 2112), alice, + "initial: term index for 'Apple' must contain alice") + + // Step 2 — batch: explicit DEL "Apple" + SET "APPLE". + // "Apple" and "APPLE" share the term token "apple" (lowercase filter). + // handleOldDeleteForSingle appends a synthetic DEL "Apple", creating 3 postings; + // InsertTokenizerIndexes does not apply the len==2 reversal — final index = DEL. + applyEdges(t, 2120, 2121, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Apple"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_DEL, + }, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("APPLE"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }, + ) + + const readTs = uint64(2122) + + // "APPLE" term-tokenises to "apple"; alice must be in the "apple" bucket because + // her tag is now "APPLE". + require.Contains(t, indexUidsForVal(t, attr, "APPLE", readTs), alice, + "index for 'APPLE' (term bucket 'apple') must contain alice after explicit-del + set batch") +} + +// TestLegacyCaseInsensitiveTokenizerDupDel mirrors Bug 5 via the legacy path to +// confirm that legacy handles explicit-DEL + SET with a shared case-insensitive +// token correctly. Legacy processes each edge independently so the SET index +// entry is generated after the DEL, leaving alice in the "apple" bucket. +func TestLegacyCaseInsensitiveTokenizerDupDel(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`casetag: string @index(term) .`), 1)) + + attr := x.AttrInRootNamespace("casetag") + const alice = uint64(10) + + applyEdgesLegacy(t, 2210, 2211, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Apple"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + applyEdgesLegacy(t, 2220, 2221, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Apple"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_DEL, + }, + &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("APPLE"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }, + ) + + const readTs = uint64(2222) + + require.Contains(t, indexUidsForVal(t, attr, "APPLE", readTs), alice, + "legacy: index for 'APPLE' (term bucket 'apple') must contain alice after explicit-del + set batch") +} + +// --------------------------------------------------------------------------- +// Issue 7: Process() channel send deadlock on goroutine error +// --------------------------------------------------------------------------- + +// TestMutationPipelineChannelDeadlock detects the deadlock that arises when a +// predicate's ProcessPredicate goroutine exits with an error before it has +// drained its edges channel, AND the main Process() loop has already filled that +// channel to capacity with remaining edges. +// +// Root cause: the edges channel has a capacity of 1000. Process() sends edges +// with a bare `pred.edges <- edge` — there is no `select` on egCtx.Done(). +// If the goroutine fails on the first edge (schema-not-found) it exits without +// consuming the 999 buffered items. The main loop then tries to send the 1001st +// edge into a full channel whose reader has gone — it blocks forever. +// +// Fix: replace `pred.edges <- edge` with: +// +// select { +// case pred.edges <- edge: +// case <-egCtx.Done(): +// return egCtx.Err() +// } +// +// This test sends exactly channelCap+2 = 1002 SET edges for a predicate with no +// registered schema. Sequence: +// 1. Main loop fills the 1000-slot buffer (edges 0-999), then blocks on edge 1000. +// 2. The goroutine reads edge 0 (freeing one slot), main loop sends edge 1000. +// 3. The goroutine calls runMutation(edge 0), fails (no schema), exits — channel +// still holds 1000 items. +// 4. Main loop tries to send edge 1001 into the full, unread channel → blocks +// forever (deadlock). +// +// With the fix, egCtx.Done() fires and Process() returns an error within +// milliseconds. Without the fix, the test blocks until the 3-second timeout. +// +// Note: channelCap+1 = 1001 edges is NOT sufficient — the goroutine reading one +// edge gives the main loop exactly one free slot to push its last edge, so the +// loop completes without ever stalling. The +2 guarantees one edge remains +// unsendable after the goroutine exits. +func TestMutationPipelineChannelDeadlock(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + // No schema registered — ProcessPredicate falls through to the legacy drain + // loop, which calls runMutation and fails on the first SET edge. + attr := x.AttrInRootNamespace("no_schema_deadlock_pred") + + const channelCap = 1000 + edges := make([]*pb.DirectedEdge, channelCap+2) + for i := range edges { + edges[i] = &pb.DirectedEdge{ + Entity: uint64(i + 1), + Attr: attr, + Value: []byte("v"), + ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + } + } + + done := make(chan error, 1) + go func() { + txn := Oracle().RegisterStartTs(2300) + mp := NewMutationPipeline(txn) + done <- mp.Process(context.Background(), edges) + }() + + select { + case err := <-done: + // Process() must return an error (schema not found), never nil. + require.Error(t, err, + "Process() must return a schema-not-found error, not nil") + case <-time.After(3 * time.Second): + t.Fatal("Process() deadlocked: channel send blocked after goroutine error — " + + "fix by using select{case pred.edges<-edge: case <-egCtx.Done():...}") + } +} + +// --------------------------------------------------------------------------- +// Issue 9: non-matching DEL is a no-op +// --------------------------------------------------------------------------- + +// TestMutationPipelineNonMatchingDelNoOp verifies that a DEL edge whose value +// does not match the committed scalar value is silently ignored — the forward +// data key and the index are left unchanged. +// +// In ProcessSingle, postings[uid] is unconditionally initialised to an empty +// PostingList before the committed-value check, so an empty delta IS written to +// Badger even for the no-op case (Issue 9 / minor inefficiency). The important +// invariant is that the observable state — the value and index — is not altered. +// +// Expected behaviour (both pipeline and legacy): +// - alice's tag is still "Alice". +// - index("Alice") still contains alice. +// - index("Blink") does not contain alice. +func TestMutationPipelineNonMatchingDelNoOp(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`tag: string @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("tag") + const alice = uint64(10) + + // Step 1 — commit initial value "Alice". + applyEdges(t, 2410, 2411, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", 2412), alice, + "initial: index('Alice') must contain alice") + + // Step 2 — DEL with wrong value: alice has "Alice", we DEL "Blink". + // ProcessSingle: DEL "Blink", old committed = "Alice", "Blink" != "Alice" → + // setPosting() is NOT called → empty delta written → no change. + applyEdges(t, 2420, 2421, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Blink"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_DEL, + }) + + const readTs = uint64(2422) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", readTs), alice, + "index('Alice') must still contain alice — non-matching DEL is a no-op") + require.NotContains(t, indexUidsForVal(t, attr, "Blink", readTs), alice, + "index('Blink') must not contain alice — no such value was ever set") +} + +// TestLegacyNonMatchingDelNoOp mirrors Issue 9 via the legacy path to confirm +// that both paths treat a non-matching DEL as a no-op. +func TestLegacyNonMatchingDelNoOp(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`tag: string @index(exact) .`), 1)) + + attr := x.AttrInRootNamespace("tag") + const alice = uint64(10) + + applyEdgesLegacy(t, 2510, 2511, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + applyEdgesLegacy(t, 2520, 2521, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Blink"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_DEL, + }) + + const readTs = uint64(2522) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", readTs), alice, + "legacy: index('Alice') must still contain alice — non-matching DEL is a no-op") + require.NotContains(t, indexUidsForVal(t, attr, "Blink", readTs), alice, + "legacy: index('Blink') must not contain alice") +} + +// --------------------------------------------------------------------------- +// Bug 10: same-value SET on already-committed scalar index entry +// --------------------------------------------------------------------------- + +// TestMutationPipelineSameValueSetKeepsIndex is a baseline regression test: +// when the schema already carries @index(term) at the time of the initial SET, +// the index entry must survive a second identical SET on the same uid. +// +// This test passes both before and after the handleOldDeleteForSingle fix, +// because the initial SET already populated the index. The companion test +// TestMutationPipelineSetThenAddIndexThenResetMirrorsAlphaFailure is the one +// that reproduces the actual alpha-level TestSchemaMutationIndexRemove +// failure — that scenario requires the SET to happen *before* the @index +// alter, so the second SET is responsible for populating the index. +// +// Legacy mirror: TestLegacySameValueSetKeepsIndex. +func TestMutationPipelineSameValueSetKeepsIndex(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @index(term) .`), 1)) + + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + applyEdges(t, 2610, 2611, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", 2612), alice, + "initial: index('Alice') must contain alice") + + applyEdges(t, 2620, 2621, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(2622) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", readTs), alice, + "index('Alice') must still contain alice after same-value SET re-commit") +} + +// TestLegacySameValueSetKeepsIndex mirrors Bug 10 via the legacy path. +func TestLegacySameValueSetKeepsIndex(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @index(term) .`), 1)) + + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + applyEdgesLegacy(t, 2710, 2711, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", 2712), alice, + "legacy initial: index('Alice') must contain alice") + + applyEdgesLegacy(t, 2720, 2721, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(2722) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", readTs), alice, + "legacy: index('Alice') must still contain alice after same-value SET re-commit") +} + +// TestMutationPipelineSetThenAddIndexThenResetMirrorsAlphaFailure is the +// regression test for the alpha-level failure TestSchemaMutationIndexRemove. +// +// Sequence: +// 1. Schema is `name: string` (no @index). +// 2. SET name "Alice" is committed. +// 3. Schema altered to add @index(term). +// 4. SET name "Alice" again (same value, but now under @index(term)). +// 5. Query the term index for "Alice" — must contain alice. +// +// Root cause (pre-fix): handleOldDeleteForSingle in posting/index.go saw that +// the new value matched the committed value and replaced postings[uid] with +// an empty PostingList. InsertTokenizerIndexes then iterated an empty list +// for that uid and emitted no index entries, so the newly-added index never +// got the "alice" → alice entry. Legacy passes because runMutation calls +// addIndexMutations per SET edge regardless of whether the value changed. +// +// Fix: handleOldDeleteForSingle now `continue`s in the same-value branch +// without clearing postings[uid]. The SET stays in place so +// InsertTokenizerIndexes / ProcessReverse can populate the new index/reverse +// edge. Index/reverse emissions on already-present entries are idempotent. +func TestMutationPipelineSetThenAddIndexThenResetMirrorsAlphaFailure(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + // Step 1: no-index schema initially. + require.NoError(t, schema.ParseBytes([]byte(`name: string .`), 1)) + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + // Step 2: SET Alice with no index. + applyEdges(t, 2810, 2811, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + // Step 3: alter — add term index. Simulate alpha's background reindex by + // flipping schema then re-running a SET (the reindex on alpha rewrites the + // index from existing data; here we replay the existing SET under the new + // schema to drive InsertTokenizerIndexes). + require.NoError(t, schema.ParseBytes([]byte(`name: string @index(term) .`), 1)) + applyEdges(t, 2820, 2821, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", 2822), alice, + "after reindex: index('Alice') must contain alice") + + // Step 4: SET Alice again, same value, index now exists. + applyEdges(t, 2830, 2831, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + const readTs = uint64(2832) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", readTs), alice, + "index('Alice') must still contain alice after same-value SET on top of @index(term)") +} + +// TestLegacySetThenAddIndexThenResetMirrorsAlphaFailure is the legacy mirror +// of the test above. Legacy is expected to pass because runMutation calls +// addIndexMutations for every SET regardless of whether the value changed. +func TestLegacySetThenAddIndexThenResetMirrorsAlphaFailure(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string .`), 1)) + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + applyEdgesLegacy(t, 2910, 2911, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.NoError(t, schema.ParseBytes([]byte(`name: string @index(term) .`), 1)) + applyEdgesLegacy(t, 2920, 2921, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("Alice"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Contains(t, indexUidsForVal(t, attr, "Alice", 2922), alice, + "legacy after reindex: index('Alice') must contain alice") +} + +// scalarValue returns the string value of the single scalar posting under +// (attr, entity) at readTs, or empty if there is no posting. +func scalarValue(t *testing.T, attr string, entity, readTs uint64) string { + t.Helper() + l, err := GetNoStore(x.DataKey(attr, entity), readTs) + require.NoError(t, err) + var out string + require.NoError(t, l.Iterate(readTs, 0, func(p *pb.Posting) error { + out = string(p.Value) + return nil + })) + return out +} + +// TestMutationPipelineDelWithWrongValueIsNoOp is the regression test for +// TestDeleteScalarValue at the alpha integration level. +// +// Sequence: +// 1. Schema: name: string @index(exact). +// 2. SET name "xxx" (committed). +// 3. DEL name "yyy" (wrong value — does not match committed "xxx"). +// 4. The data must still read as "xxx" — a DEL whose value does not match +// the committed value is a no-op per Dgraph semantics. +// +// Bug: ProcessSingle accumulates `postings[alice] = &pb.PostingList{}` for the +// non-matching DEL (sameTarget is false, so setPosting is not called). The +// data-write loop then calls AddDelta(dataKey, emptyPL) which clobbers the +// in-memory list's mutable layer via setCurrentEntries → the next read at +// readTs > commitTs surfaces an empty list. +func TestMutationPipelineDelWithWrongValueIsNoOp(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @index(exact) .`), 1)) + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + applyEdges(t, 3010, 3011, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("xxx"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + require.Equal(t, "xxx", scalarValue(t, attr, alice, 3012), + "sanity: committed value should be xxx") + + applyEdges(t, 3020, 3021, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("yyy"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_DEL, + }) + + require.Equal(t, "xxx", scalarValue(t, attr, alice, 3022), + "DEL with wrong value must be a no-op — name still xxx") +} + +// TestLegacyDelWithWrongValueIsNoOp mirrors the above via the legacy path. +func TestLegacyDelWithWrongValueIsNoOp(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @index(exact) .`), 1)) + attr := x.AttrInRootNamespace("name") + const alice = uint64(10) + + applyEdgesLegacy(t, 3110, 3111, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("xxx"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + + applyEdgesLegacy(t, 3120, 3121, &pb.DirectedEdge{ + Entity: alice, Attr: attr, + Value: []byte("yyy"), ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_DEL, + }) + + require.Equal(t, "xxx", scalarValue(t, attr, alice, 3122), + "legacy: DEL with wrong value must be a no-op") +} + +// TestMutationPipelineDelWrongValueTwoUidsCloseMatch mirrors TestDeleteScalarValue's +// exact shape: two uids both set to "xxx", then DEL on one of them with value "yyy". +// The DEL must not delete; the SET on the other uid must remain. +func TestMutationPipelineDelWrongValueTwoUidsCloseMatch(t *testing.T) { + require.NoError(t, pstore.DropAll()) + MemLayerInstance.clear() + + require.NoError(t, schema.ParseBytes([]byte(`name: string @index(exact) .`), 1)) + attr := x.AttrInRootNamespace("name") + const ( + a = uint64(0x12345) + b = uint64(0x12346) + ) + + applyEdges(t, 3210, 3211, + &pb.DirectedEdge{Entity: a, Attr: attr, Value: []byte("xxx"), ValueType: pb.Posting_STRING, Op: pb.DirectedEdge_SET}, + &pb.DirectedEdge{Entity: b, Attr: attr, Value: []byte("xxx"), ValueType: pb.Posting_STRING, Op: pb.DirectedEdge_SET}, + ) + + require.Equal(t, "xxx", scalarValue(t, attr, a, 3212), "sanity: a=xxx") + require.Equal(t, "xxx", scalarValue(t, attr, b, 3212), "sanity: b=xxx") + + applyEdges(t, 3220, 3221, + &pb.DirectedEdge{Entity: a, Attr: attr, Value: []byte("yyy"), ValueType: pb.Posting_STRING, Op: pb.DirectedEdge_DEL}, + ) + + const readTs = uint64(3222) + require.Equal(t, "xxx", scalarValue(t, attr, a, readTs), + "DEL with wrong value must be a no-op — a still xxx") + require.Equal(t, "xxx", scalarValue(t, attr, b, readTs), + "DEL on a must not touch b") + require.Contains(t, indexUidsForVal(t, attr, "xxx", readTs), a, + "index('xxx') must still contain a") + require.Contains(t, indexUidsForVal(t, attr, "xxx", readTs), b, + "index('xxx') must still contain b") +} diff --git a/query/upgrade_test.go b/query/upgrade_test.go index 9421e83fca9..dcec57e4f8c 100644 --- a/query/upgrade_test.go +++ b/query/upgrade_test.go @@ -50,7 +50,7 @@ func TestMain(m *testing.M) { WithReplicas(1).WithACL(time.Hour).WithVersion(uc.Before) c, err := dgraphtest.NewLocalCluster(conf) x.Panic(err) - defer func() { c.Cleanup(code != 0) }() + defer func() { c.Cleanup(true) }() x.Panic(c.Start()) hc, err := c.HTTPClient() diff --git a/schema/schema.go b/schema/schema.go index f8852d2aa0e..b3beb650a27 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -662,8 +662,7 @@ func initialTypesInternal(namespace uint64, all bool) []*pb.TypeUpdate { ValueType: pb.Posting_STRING, }, }, - }, - &pb.TypeUpdate{ + }, &pb.TypeUpdate{ TypeName: "dgraph.graphql.persisted_query", Fields: []*pb.SchemaUpdate{ { @@ -693,24 +692,23 @@ func initialTypesInternal(namespace uint64, all bool) []*pb.TypeUpdate { if all || x.WorkerConfig.AclEnabled { // These type definitions are required for deleteUser and deleteGroup GraphQL API to work // properly. - initialTypes = append(initialTypes, - &pb.TypeUpdate{ - TypeName: "dgraph.type.User", - Fields: []*pb.SchemaUpdate{ - { - Predicate: "dgraph.xid", - ValueType: pb.Posting_STRING, - }, - { - Predicate: "dgraph.password", - ValueType: pb.Posting_PASSWORD, - }, - { - Predicate: "dgraph.user.group", - ValueType: pb.Posting_UID, - }, + initialTypes = append(initialTypes, &pb.TypeUpdate{ + TypeName: "dgraph.type.User", + Fields: []*pb.SchemaUpdate{ + { + Predicate: "dgraph.xid", + ValueType: pb.Posting_STRING, + }, + { + Predicate: "dgraph.password", + ValueType: pb.Posting_PASSWORD, + }, + { + Predicate: "dgraph.user.group", + ValueType: pb.Posting_UID, }, }, + }, &pb.TypeUpdate{ TypeName: "dgraph.type.Group", Fields: []*pb.SchemaUpdate{ @@ -767,36 +765,31 @@ func CompleteInitialSchema(namespace uint64) []*pb.SchemaUpdate { func initialSchemaInternal(namespace uint64, all bool) []*pb.SchemaUpdate { var initialSchema []*pb.SchemaUpdate - initialSchema = append(initialSchema, []*pb.SchemaUpdate{ - { + initialSchema = append(initialSchema, + &pb.SchemaUpdate{ Predicate: "dgraph.type", ValueType: pb.Posting_STRING, Directive: pb.SchemaUpdate_INDEX, Tokenizer: []string{"exact"}, List: true, - }, - { + }, &pb.SchemaUpdate{ Predicate: "dgraph.drop.op", ValueType: pb.Posting_STRING, - }, - { + }, &pb.SchemaUpdate{ Predicate: "dgraph.graphql.schema", ValueType: pb.Posting_STRING, - }, - { + }, &pb.SchemaUpdate{ Predicate: "dgraph.graphql.xid", ValueType: pb.Posting_STRING, Directive: pb.SchemaUpdate_INDEX, Tokenizer: []string{"exact"}, Upsert: true, - }, - { + }, &pb.SchemaUpdate{ Predicate: "dgraph.graphql.p_query", ValueType: pb.Posting_STRING, Directive: pb.SchemaUpdate_INDEX, Tokenizer: []string{"sha256"}, - }, - }...) + }) if namespace == x.RootNamespace { initialSchema = append(initialSchema, []*pb.SchemaUpdate{ diff --git a/systest/21million/queries/query-073 b/systest/21million/queries/query-073 new file mode 100644 index 00000000000..670cb00f07d --- /dev/null +++ b/systest/21million/queries/query-073 @@ -0,0 +1,2378 @@ +{ + q(func: type(Genre), orderasc: name@en) @filter(gt(count(~genre), 0)) { + name@en + n_films: count(~genre) + } +} +--- +{ + "q": [ + { + "name@en": "/m/04rlf", + "n_films": 2 + }, + { + "name@en": "3D film", + "n_films": 4 + }, + { + "name@en": "Abstract animation", + "n_films": 6 + }, + { + "name@en": "Absurdism", + "n_films": 74 + }, + { + "name@en": "Acid Western", + "n_films": 8 + }, + { + "name@en": "Action", + "n_films": 48 + }, + { + "name@en": "Action", + "n_films": 1 + }, + { + "name@en": "Action Comedy", + "n_films": 149 + }, + { + "name@en": "Action Film", + "n_films": 17719 + }, + { + "name@en": "Action Thriller", + "n_films": 588 + }, + { + "name@en": "Action/Adventure", + "n_films": 5731 + }, + { + "name@en": "Adevnture", + "n_films": 1 + }, + { + "name@en": "Adult", + "n_films": 5 + }, + { + "name@en": "Adventure", + "n_films": 20 + }, + { + "name@en": "Adventure Comedy", + "n_films": 123 + }, + { + "name@en": "Adventure Film", + "n_films": 9367 + }, + { + "name@en": "Adventure game", + "n_films": 57 + }, + { + "name@en": "Adventure Travel", + "n_films": 8 + }, + { + "name@en": "African-American Comedies", + "n_films": 1 + }, + { + "name@en": "Ages 0-2", + "n_films": 2 + }, + { + "name@en": "Ages 2-4", + "n_films": 1 + }, + { + "name@en": "Ages 5-7", + "n_films": 1 + }, + { + "name@en": "Airplanes and airports", + "n_films": 30 + }, + { + "name@en": "Albinism in popular culture", + "n_films": 11 + }, + { + "name@en": "Alien Film", + "n_films": 97 + }, + { + "name@en": "Alien invasion", + "n_films": 4 + }, + { + "name@en": "Alien Sci-Fi", + "n_films": 2 + }, + { + "name@en": "Alternate history", + "n_films": 4 + }, + { + "name@en": "Alternative Health", + "n_films": 4 + }, + { + "name@en": "Amateur film", + "n_films": 1 + }, + { + "name@en": "Ambiance", + "n_films": 11 + }, + { + "name@en": "Animal", + "n_films": 54 + }, + { + "name@en": "Animal Picture", + "n_films": 193 + }, + { + "name@en": "Animated cartoon", + "n_films": 132 + }, + { + "name@en": "Animated documentary", + "n_films": 9 + }, + { + "name@en": "Animated Musical", + "n_films": 42 + }, + { + "name@en": "Animation", + "n_films": 12799 + }, + { + "name@en": "Anime", + "n_films": 1106 + }, + { + "name@en": "Anime Sci-Fi", + "n_films": 4 + }, + { + "name@en": "Anthology", + "n_films": 14 + }, + { + "name@en": "Anthology film", + "n_films": 2 + }, + { + "name@en": "Anti-McCarthyist", + "n_films": 1 + }, + { + "name@en": "Anti-war film", + "n_films": 43 + }, + { + "name@en": "Anti-war movement", + "n_films": 2 + }, + { + "name@en": "Arab cinema", + "n_films": 1 + }, + { + "name@en": "Archaeology", + "n_films": 8 + }, + { + "name@en": "Architecture & Design", + "n_films": 11 + }, + { + "name@en": "Archives and records", + "n_films": 5 + }, + { + "name@en": "Armchair Travel", + "n_films": 25 + }, + { + "name@en": "Art history", + "n_films": 18 + }, + { + "name@en": "Artistic gymnastics", + "n_films": 2 + }, + { + "name@en": "Arts and Culture", + "n_films": 395 + }, + { + "name@en": "Assamese cinema", + "n_films": 10 + }, + { + "name@en": "Auto racing", + "n_films": 210 + }, + { + "name@en": "Avant-garde", + "n_films": 152 + }, + { + "name@en": "B movie", + "n_films": 1059 + }, + { + "name@en": "B-boying", + "n_films": 2 + }, + { + "name@en": "B-Western", + "n_films": 86 + }, + { + "name@en": "Backstage", + "n_films": 1 + }, + { + "name@en": "Backstage Musical", + "n_films": 7246 + }, + { + "name@en": "Ballet", + "n_films": 10 + }, + { + "name@en": "BDSM", + "n_films": 3 + }, + { + "name@en": "Beach Film", + "n_films": 21 + }, + { + "name@en": "Beach party film", + "n_films": 3 + }, + { + "name@en": "Beauty & Fashion", + "n_films": 15 + }, + { + "name@en": "Bedroom farce", + "n_films": 1 + }, + { + "name@en": "Bengali Cinema", + "n_films": 854 + }, + { + "name@en": "Best of compilation", + "n_films": 1 + }, + { + "name@en": "Bhojpuri cinema", + "n_films": 4 + }, + { + "name@en": "Biker Film", + "n_films": 38 + }, + { + "name@en": "Biographical Documentaries", + "n_films": 27 + }, + { + "name@en": "Biographical film", + "n_films": 5968 + }, + { + "name@en": "Biography", + "n_films": 67 + }, + { + "name@en": "Biological Sciences", + "n_films": 15 + }, + { + "name@en": "Bisexual pornography", + "n_films": 12 + }, + { + "name@en": "Black comedy", + "n_films": 1313 + }, + { + "name@en": "Black-and-white", + "n_films": 7780 + }, + { + "name@en": "Blaxploitation film", + "n_films": 121 + }, + { + "name@en": "Bloopers & Candid Camera", + "n_films": 7 + }, + { + "name@en": "Bollywood", + "n_films": 5119 + }, + { + "name@en": "British Cinema", + "n_films": 10 + }, + { + "name@en": "British Empire Film", + "n_films": 21 + }, + { + "name@en": "British New Wave", + "n_films": 12 + }, + { + "name@en": "Bruceploitation", + "n_films": 4 + }, + { + "name@en": "Buddy cop film", + "n_films": 11 + }, + { + "name@en": "Buddy film", + "n_films": 357 + }, + { + "name@en": "Business", + "n_films": 2 + }, + { + "name@en": "Business", + "n_films": 17 + }, + { + "name@en": "C-Movie", + "n_films": 1 + }, + { + "name@en": "Camp", + "n_films": 2 + }, + { + "name@en": "Cannibal film", + "n_films": 1 + }, + { + "name@en": "Caper story", + "n_films": 96 + }, + { + "name@en": "Careers", + "n_films": 4 + }, + { + "name@en": "Cartoon", + "n_films": 374 + }, + { + "name@en": "Cavalry Film", + "n_films": 19 + }, + { + "name@en": "Chase Movie", + "n_films": 89 + }, + { + "name@en": "Chick flick", + "n_films": 2 + }, + { + "name@en": "Childhood Drama", + "n_films": 132 + }, + { + "name@en": "Children's Fantasy", + "n_films": 229 + }, + { + "name@en": "Children's Issues", + "n_films": 14 + }, + { + "name@en": "Children's television series", + "n_films": 1 + }, + { + "name@en": "Children's/Family", + "n_films": 884 + }, + { + "name@en": "Chinese Movies", + "n_films": 2446 + }, + { + "name@en": "Chivalric romance", + "n_films": 2 + }, + { + "name@en": "Christian film", + "n_films": 122 + }, + { + "name@en": "Christian film industry", + "n_films": 3 + }, + { + "name@en": "Christianity", + "n_films": 5 + }, + { + "name@en": "Christmas", + "n_films": 1 + }, + { + "name@en": "Christmas movie", + "n_films": 94 + }, + { + "name@en": "Christmas special", + "n_films": 3 + }, + { + "name@en": "Cin\u00e9ma v\u00e9rit\u00e9", + "n_films": 7 + }, + { + "name@en": "Clay animation", + "n_films": 7 + }, + { + "name@en": "Closeted", + "n_films": 1 + }, + { + "name@en": "Collage", + "n_films": 2 + }, + { + "name@en": "College life", + "n_films": 1 + }, + { + "name@en": "Color motion picture film", + "n_films": 2 + }, + { + "name@en": "Combat Films", + "n_films": 93 + }, + { + "name@en": "Comedy", + "n_films": 41070 + }, + { + "name@en": "Comedy of Errors", + "n_films": 182 + }, + { + "name@en": "Comedy of manners", + "n_films": 218 + }, + { + "name@en": "Comedy Thriller", + "n_films": 93 + }, + { + "name@en": "Comedy Western", + "n_films": 241 + }, + { + "name@en": "Comedy-drama", + "n_films": 2173 + }, + { + "name@en": "Coming of age", + "n_films": 781 + }, + { + "name@en": "Coming out", + "n_films": 13 + }, + { + "name@en": "Coming-of-age story", + "n_films": 277 + }, + { + "name@en": "Commedia all'italiana", + "n_films": 2 + }, + { + "name@en": "Commedia sexy all'italiana", + "n_films": 2 + }, + { + "name@en": "Compilation film", + "n_films": 15 + }, + { + "name@en": "Computer Animation", + "n_films": 197 + }, + { + "name@en": "Computers", + "n_films": 13 + }, + { + "name@en": "Concert", + "n_films": 4 + }, + { + "name@en": "Concert film", + "n_films": 3050 + }, + { + "name@en": "Conspiracy fiction", + "n_films": 1 + }, + { + "name@en": "Cooking & Food", + "n_films": 6 + }, + { + "name@en": "Costume Adventure", + "n_films": 80 + }, + { + "name@en": "Costume drama", + "n_films": 359 + }, + { + "name@en": "Costume Horror", + "n_films": 42 + }, + { + "name@en": "Courtroom Comedy", + "n_films": 6 + }, + { + "name@en": "Creature Film", + "n_films": 306 + }, + { + "name@en": "Crime", + "n_films": 224 + }, + { + "name@en": "Crime Comedy", + "n_films": 272 + }, + { + "name@en": "Crime Drama", + "n_films": 479 + }, + { + "name@en": "Crime Fiction", + "n_films": 13019 + }, + { + "name@en": "Crime film", + "n_films": 42 + }, + { + "name@en": "Crime Thriller", + "n_films": 2541 + }, + { + "name@en": "Cult film", + "n_films": 937 + }, + { + "name@en": "Culture & Society", + "n_films": 895 + }, + { + "name@en": "Current affairs", + "n_films": 1 + }, + { + "name@en": "Cyberpunk", + "n_films": 9 + }, + { + "name@en": "Cycling", + "n_films": 1 + }, + { + "name@en": "Czechoslovak New Wave", + "n_films": 6 + }, + { + "name@en": "Damsel in distress", + "n_films": 1 + }, + { + "name@en": "Dance film", + "n_films": 986 + }, + { + "name@en": "Demonic child", + "n_films": 5 + }, + { + "name@en": "Detective fiction", + "n_films": 520 + }, + { + "name@en": "Diet & Nutrition", + "n_films": 8 + }, + { + "name@en": "Direct-to-video", + "n_films": 51 + }, + { + "name@en": "Disaster Film", + "n_films": 318 + }, + { + "name@en": "Docmentary", + "n_films": 1 + }, + { + "name@en": "Docudrama", + "n_films": 477 + }, + { + "name@en": "Docufiction", + "n_films": 28 + }, + { + "name@en": "Documentary film", + "n_films": 31370 + }, + { + "name@en": "Documentation", + "n_films": 1 + }, + { + "name@en": "Dogme 95", + "n_films": 77 + }, + { + "name@en": "Dokumentarfilm", + "n_films": 1 + }, + { + "name@en": "Domaci", + "n_films": 1 + }, + { + "name@en": "Domestic Comedy", + "n_films": 179 + }, + { + "name@en": "Doomsday film", + "n_films": 79 + }, + { + "name@en": "Drama", + "n_films": 238 + }, + { + "name@en": "Drama", + "n_films": 76252 + }, + { + "name@en": "Dystopia", + "n_films": 62 + }, + { + "name@en": "Early Black Cinema", + "n_films": 7 + }, + { + "name@en": "East Asian cinema", + "n_films": 3747 + }, + { + "name@en": "Education", + "n_films": 26 + }, + { + "name@en": "Educational & Guidance", + "n_films": 1 + }, + { + "name@en": "Educational entertainment", + "n_films": 1 + }, + { + "name@en": "Educational film", + "n_films": 3852 + }, + { + "name@en": "Edupunk", + "n_films": 1 + }, + { + "name@en": "Ensemble Film", + "n_films": 328 + }, + { + "name@en": "Environment", + "n_films": 11 + }, + { + "name@en": "Environmental Science", + "n_films": 36 + }, + { + "name@en": "Epic film", + "n_films": 318 + }, + { + "name@en": "Epic Western", + "n_films": 16 + }, + { + "name@en": "Equestrianism", + "n_films": 1 + }, + { + "name@en": "Erotic Drama", + "n_films": 132 + }, + { + "name@en": "Erotic Science Fiction", + "n_films": 1 + }, + { + "name@en": "Erotic thriller", + "n_films": 471 + }, + { + "name@en": "Erotica", + "n_films": 719 + }, + { + "name@en": "Escape Film", + "n_films": 45 + }, + { + "name@en": "Essay Film", + "n_films": 25 + }, + { + "name@en": "Estonian animation", + "n_films": 3 + }, + { + "name@en": "Ethnofiction", + "n_films": 2 + }, + { + "name@en": "Ethnographic film", + "n_films": 4 + }, + { + "name@en": "Eurospy film", + "n_films": 1 + }, + { + "name@en": "Eurowestern", + "n_films": 5 + }, + { + "name@en": "Evil clown", + "n_films": 3 + }, + { + "name@en": "Existentialism", + "n_films": 16 + }, + { + "name@en": "Experimental film", + "n_films": 1563 + }, + { + "name@en": "Exploitation film", + "n_films": 43 + }, + { + "name@en": "Expressionism", + "n_films": 3 + }, + { + "name@en": "Extraterrestrial life", + "n_films": 1 + }, + { + "name@en": "Extreme Sports", + "n_films": 174 + }, + { + "name@en": "Fairy tale", + "n_films": 80 + }, + { + "name@en": "Family", + "n_films": 11267 + }, + { + "name@en": "Family", + "n_films": 11 + }, + { + "name@en": "Family Comedies", + "n_films": 3 + }, + { + "name@en": "Family Drama", + "n_films": 1024 + }, + { + "name@en": "Family-Oriented Adventure", + "n_films": 237 + }, + { + "name@en": "Fan film", + "n_films": 43 + }, + { + "name@en": "Fantasy", + "n_films": 466 + }, + { + "name@en": "Fantasy", + "n_films": 1 + }, + { + "name@en": "Fantasy", + "n_films": 5854 + }, + { + "name@en": "Fantasy", + "n_films": 1 + }, + { + "name@en": "Fantasy Adventure", + "n_films": 162 + }, + { + "name@en": "Fantasy Comedy", + "n_films": 171 + }, + { + "name@en": "Fantasy Drama", + "n_films": 15 + }, + { + "name@en": "Farce", + "n_films": 293 + }, + { + "name@en": "Fashion", + "n_films": 1 + }, + { + "name@en": "Feature film", + "n_films": 26 + }, + { + "name@en": "Female buddy film", + "n_films": 4 + }, + { + "name@en": "Feminist Film", + "n_films": 80 + }, + { + "name@en": "Fiction", + "n_films": 71 + }, + { + "name@en": "Fiction", + "n_films": 1 + }, + { + "name@en": "Fictional film", + "n_films": 43 + }, + { + "name@en": "Filipino Movies", + "n_films": 18 + }, + { + "name@en": "Film & Television History", + "n_films": 428 + }, + { + "name@en": "Film \u00e0 clef", + "n_films": 29 + }, + { + "name@en": "Film adaptation", + "n_films": 1448 + }, + { + "name@en": "Film noir", + "n_films": 757 + }, + { + "name@en": "Film-Opera", + "n_films": 5 + }, + { + "name@en": "Film-poem", + "n_films": 1 + }, + { + "name@en": "Filmed Play", + "n_films": 18 + }, + { + "name@en": "Films with live action and animation", + "n_films": 8 + }, + { + "name@en": "Finance & Investing", + "n_films": 7 + }, + { + "name@en": "Flash animation", + "n_films": 1 + }, + { + "name@en": "Foreign legion", + "n_films": 8 + }, + { + "name@en": "Found footage", + "n_films": 10 + }, + { + "name@en": "French New Wave", + "n_films": 1 + }, + { + "name@en": "Future history", + "n_films": 1 + }, + { + "name@en": "Future noir", + "n_films": 18 + }, + { + "name@en": "Game show", + "n_films": 4 + }, + { + "name@en": "Gangster Film", + "n_films": 426 + }, + { + "name@en": "Gay", + "n_films": 306 + }, + { + "name@en": "Gay & Lesbian Films", + "n_films": 3 + }, + { + "name@en": "Gay Interest", + "n_films": 300 + }, + { + "name@en": "Gay pornography", + "n_films": 10970 + }, + { + "name@en": "Gay Themed", + "n_films": 467 + }, + { + "name@en": "Gender Issues", + "n_films": 107 + }, + { + "name@en": "Giallo", + "n_films": 20 + }, + { + "name@en": "Glamorized Spy Film", + "n_films": 47 + }, + { + "name@en": "Goat gland film", + "n_films": 3 + }, + { + "name@en": "Gothic Film", + "n_films": 52 + }, + { + "name@en": "Graphic & Applied Arts", + "n_films": 32 + }, + { + "name@en": "Gross out", + "n_films": 45 + }, + { + "name@en": "Gujarati cinema", + "n_films": 1 + }, + { + "name@en": "Gulf War", + "n_films": 4 + }, + { + "name@en": "Hagiography", + "n_films": 26 + }, + { + "name@en": "Hard rock", + "n_films": 1 + }, + { + "name@en": "Hardcore pornography", + "n_films": 278 + }, + { + "name@en": "Hardcore punk", + "n_films": 1 + }, + { + "name@en": "Haunted House Film", + "n_films": 66 + }, + { + "name@en": "Health & Fitness", + "n_films": 218 + }, + { + "name@en": "Heaven-Can-Wait Fantasies", + "n_films": 24 + }, + { + "name@en": "Heavenly Comedy", + "n_films": 30 + }, + { + "name@en": "Heavenly Drama", + "n_films": 2 + }, + { + "name@en": "Heavy metal", + "n_films": 1 + }, + { + "name@en": "Heist film", + "n_films": 203 + }, + { + "name@en": "Hentai", + "n_films": 35 + }, + { + "name@en": "Hip hop film", + "n_films": 770 + }, + { + "name@en": "Historical drama", + "n_films": 1038 + }, + { + "name@en": "Historical Epic", + "n_films": 64 + }, + { + "name@en": "Historical fiction", + "n_films": 2095 + }, + { + "name@en": "Historical period drama", + "n_films": 1404 + }, + { + "name@en": "History", + "n_films": 2272 + }, + { + "name@en": "Hobbies and interests", + "n_films": 167 + }, + { + "name@en": "Holiday", + "n_films": 4 + }, + { + "name@en": "Holiday Film", + "n_films": 97 + }, + { + "name@en": "Home movies", + "n_films": 2 + }, + { + "name@en": "Horor", + "n_films": 1 + }, + { + "name@en": "Horror", + "n_films": 10511 + }, + { + "name@en": "Horror comedy", + "n_films": 794 + }, + { + "name@en": "Human sexuality", + "n_films": 5 + }, + { + "name@en": "Hybrid Western", + "n_films": 41 + }, + { + "name@en": "Illnesses & Disabilities", + "n_films": 65 + }, + { + "name@en": "Incest pornography", + "n_films": 2 + }, + { + "name@en": "Independent Dramas", + "n_films": 2 + }, + { + "name@en": "Indian Movies", + "n_films": 8 + }, + { + "name@en": "Indian Western", + "n_films": 26 + }, + { + "name@en": "Indie film", + "n_films": 8644 + }, + { + "name@en": "Informational", + "n_films": 6 + }, + { + "name@en": "Inspirational Drama", + "n_films": 39 + }, + { + "name@en": "inspirational film", + "n_films": 1 + }, + { + "name@en": "Instrumental Music", + "n_films": 56 + }, + { + "name@en": "International", + "n_films": 1 + }, + { + "name@en": "Interpersonal Relationships", + "n_films": 55 + }, + { + "name@en": "Inventions & Innovations", + "n_films": 16 + }, + { + "name@en": "Japanese", + "n_films": 1 + }, + { + "name@en": "Japanese horror", + "n_films": 3 + }, + { + "name@en": "Japanese Movies", + "n_films": 3257 + }, + { + "name@en": "Jazz & Easy Listening", + "n_films": 1 + }, + { + "name@en": "Jazz & Modern Dance", + "n_films": 16 + }, + { + "name@en": "Jidaigeki", + "n_films": 9 + }, + { + "name@en": "Journalism", + "n_films": 9 + }, + { + "name@en": "Jukebox musical", + "n_films": 6 + }, + { + "name@en": "Jungle Film", + "n_films": 66 + }, + { + "name@en": "Juvenile Delinquency Film", + "n_films": 49 + }, + { + "name@en": "Kafkaesque", + "n_films": 3 + }, + { + "name@en": "Kaiju", + "n_films": 7 + }, + { + "name@en": "Kannada", + "n_films": 10 + }, + { + "name@en": "Kids' TV", + "n_films": 3 + }, + { + "name@en": "Kitchen sink realism", + "n_films": 15 + }, + { + "name@en": "Korean drama", + "n_films": 109 + }, + { + "name@en": "Korean horror", + "n_films": 3 + }, + { + "name@en": "Language & Literature", + "n_films": 48 + }, + { + "name@en": "Language Instruction", + "n_films": 1 + }, + { + "name@en": "Latin American cinema", + "n_films": 13 + }, + { + "name@en": "Latino", + "n_films": 2 + }, + { + "name@en": "Law & Crime", + "n_films": 84 + }, + { + "name@en": "Legal drama", + "n_films": 9 + }, + { + "name@en": "Leisure Arts", + "n_films": 15 + }, + { + "name@en": "LGBT", + "n_films": 3605 + }, + { + "name@en": "Libraries and librarians", + "n_films": 5 + }, + { + "name@en": "Lifestyle", + "n_films": 1 + }, + { + "name@en": "Linguistics", + "n_films": 5 + }, + { + "name@en": "Literary Studies", + "n_films": 33 + }, + { + "name@en": "Live action", + "n_films": 15 + }, + { + "name@en": "Lost film", + "n_films": 17 + }, + { + "name@en": "Mafia", + "n_films": 1 + }, + { + "name@en": "Mafia comedy", + "n_films": 3 + }, + { + "name@en": "Magic realism", + "n_films": 4 + }, + { + "name@en": "Malayalam Cinema", + "n_films": 2155 + }, + { + "name@en": "Male bonding", + "n_films": 2 + }, + { + "name@en": "Marathi cinema", + "n_films": 286 + }, + { + "name@en": "Marriage & Commitment", + "n_films": 10 + }, + { + "name@en": "Marriage Drama", + "n_films": 160 + }, + { + "name@en": "Martial Arts Film", + "n_films": 1944 + }, + { + "name@en": "Masala film", + "n_films": 6 + }, + { + "name@en": "Mashup", + "n_films": 1 + }, + { + "name@en": "Mecha", + "n_films": 21 + }, + { + "name@en": "Mecha anime and manga", + "n_films": 3 + }, + { + "name@en": "Media Satire", + "n_films": 71 + }, + { + "name@en": "Media studies", + "n_films": 45 + }, + { + "name@en": "Medical fiction", + "n_films": 74 + }, + { + "name@en": "Medieval fantasy", + "n_films": 1 + }, + { + "name@en": "Melodrama", + "n_films": 1206 + }, + { + "name@en": "Metalcore", + "n_films": 2 + }, + { + "name@en": "Miniseries", + "n_films": 1 + }, + { + "name@en": "Mistery", + "n_films": 1 + }, + { + "name@en": "Mockbuster", + "n_films": 13 + }, + { + "name@en": "Mockumentary", + "n_films": 280 + }, + { + "name@en": "Mondo film", + "n_films": 11 + }, + { + "name@en": "Monster", + "n_films": 104 + }, + { + "name@en": "Monster movie", + "n_films": 252 + }, + { + "name@en": "Montage", + "n_films": 1 + }, + { + "name@en": "Motion capture", + "n_films": 1 + }, + { + "name@en": "Motorsport", + "n_films": 15 + }, + { + "name@en": "Movies About Gladiators", + "n_films": 2 + }, + { + "name@en": "Multimedia", + "n_films": 2 + }, + { + "name@en": "Mumblecore", + "n_films": 8 + }, + { + "name@en": "Murder-mystery", + "n_films": 5 + }, + { + "name@en": "Museums and art galleries", + "n_films": 1 + }, + { + "name@en": "Music", + "n_films": 8705 + }, + { + "name@en": "Music video", + "n_films": 183 + }, + { + "name@en": "Musical comedy", + "n_films": 2681 + }, + { + "name@en": "Musical Drama", + "n_films": 2841 + }, + { + "name@en": "Musical theatre", + "n_films": 2 + }, + { + "name@en": "Mystery", + "n_films": 6520 + }, + { + "name@en": "Mystery film", + "n_films": 23 + }, + { + "name@en": "Mythic fiction", + "n_films": 5 + }, + { + "name@en": "Mythological Fantasy", + "n_films": 23 + }, + { + "name@en": "Natural horror film", + "n_films": 153 + }, + { + "name@en": "Nature", + "n_films": 73 + }, + { + "name@en": "Nazi exploitation", + "n_films": 4 + }, + { + "name@en": "Neo-noir", + "n_films": 58 + }, + { + "name@en": "New Queer Cinema", + "n_films": 1 + }, + { + "name@en": "News", + "n_films": 76 + }, + { + "name@en": "Newsreel", + "n_films": 7 + }, + { + "name@en": "Ninja movie", + "n_films": 1 + }, + { + "name@en": "No Wave Cinema", + "n_films": 1 + }, + { + "name@en": "Non-fiction", + "n_films": 40 + }, + { + "name@en": "Northern", + "n_films": 48 + }, + { + "name@en": "Nuclear warfare", + "n_films": 1 + }, + { + "name@en": "Nudie film", + "n_films": 1 + }, + { + "name@en": "Nunsploitation", + "n_films": 14 + }, + { + "name@en": "Opera", + "n_films": 51 + }, + { + "name@en": "Operetta", + "n_films": 6 + }, + { + "name@en": "Operetta film", + "n_films": 5 + }, + { + "name@en": "Ostern", + "n_films": 5 + }, + { + "name@en": "Outdoor & Mountain Sports", + "n_films": 8 + }, + { + "name@en": "Outlaw", + "n_films": 2 + }, + { + "name@en": "Paracinema", + "n_films": 1 + }, + { + "name@en": "Paranormal", + "n_films": 6 + }, + { + "name@en": "Parody", + "n_films": 1229 + }, + { + "name@en": "Parody film", + "n_films": 19 + }, + { + "name@en": "Part-talkie", + "n_films": 5 + }, + { + "name@en": "Patriotic film", + "n_films": 3 + }, + { + "name@en": "Period Horror", + "n_films": 1 + }, + { + "name@en": "Pink film", + "n_films": 53 + }, + { + "name@en": "Point of view shot", + "n_films": 2 + }, + { + "name@en": "Police", + "n_films": 1 + }, + { + "name@en": "Police Comedy", + "n_films": 26 + }, + { + "name@en": "Police procedural", + "n_films": 2 + }, + { + "name@en": "Political cinema", + "n_films": 855 + }, + { + "name@en": "Political Documetary", + "n_films": 2 + }, + { + "name@en": "Political drama", + "n_films": 1386 + }, + { + "name@en": "Political fiction", + "n_films": 4 + }, + { + "name@en": "Political satire", + "n_films": 73 + }, + { + "name@en": "Political thriller", + "n_films": 282 + }, + { + "name@en": "Politics", + "n_films": 1 + }, + { + "name@en": "Poliziotteschi", + "n_films": 42 + }, + { + "name@en": "Pop rock", + "n_films": 1 + }, + { + "name@en": "Pornochanchada", + "n_films": 1 + }, + { + "name@en": "Pornographic film", + "n_films": 6516 + }, + { + "name@en": "Pornography", + "n_films": 2 + }, + { + "name@en": "Post-Apocalyptic fiction", + "n_films": 30 + }, + { + "name@en": "Prison escape", + "n_films": 2 + }, + { + "name@en": "Prison film", + "n_films": 143 + }, + { + "name@en": "Propaganda film", + "n_films": 221 + }, + { + "name@en": "Psycho film", + "n_films": 1 + }, + { + "name@en": "Psycho-biddy", + "n_films": 13 + }, + { + "name@en": "Psychological Drama", + "n_films": 47 + }, + { + "name@en": "Psychological horror", + "n_films": 21 + }, + { + "name@en": "Psychological thriller", + "n_films": 1746 + }, + { + "name@en": "Public information film", + "n_films": 14 + }, + { + "name@en": "Punk rock", + "n_films": 395 + }, + { + "name@en": "Puppetoon", + "n_films": 1 + }, + { + "name@en": "Race film", + "n_films": 13 + }, + { + "name@en": "Rape and revenge film", + "n_films": 2 + }, + { + "name@en": "Real Robot", + "n_films": 2 + }, + { + "name@en": "Reality film", + "n_films": 4 + }, + { + "name@en": "Reality television", + "n_films": 30 + }, + { + "name@en": "Reboot", + "n_films": 5 + }, + { + "name@en": "Religion", + "n_films": 21 + }, + { + "name@en": "Religious Film", + "n_films": 170 + }, + { + "name@en": "Remake", + "n_films": 53 + }, + { + "name@en": "Revenge", + "n_films": 5 + }, + { + "name@en": "Revisionist Fairy Tale", + "n_films": 1 + }, + { + "name@en": "Revisionist Western", + "n_films": 68 + }, + { + "name@en": "Road movie", + "n_films": 399 + }, + { + "name@en": "Road-Horror", + "n_films": 16 + }, + { + "name@en": "Roadshow theatrical release", + "n_films": 24 + }, + { + "name@en": "Roadshow/Carny", + "n_films": 4 + }, + { + "name@en": "Rock & Pop", + "n_films": 1 + }, + { + "name@en": "Rockumentary", + "n_films": 1077 + }, + { + "name@en": "Romance", + "n_films": 4 + }, + { + "name@en": "Romance", + "n_films": 39 + }, + { + "name@en": "Romance Film", + "n_films": 21094 + }, + { + "name@en": "Romantic comedy", + "n_films": 3602 + }, + { + "name@en": "Romantic Drama", + "n_films": 1 + }, + { + "name@en": "Romantic fantasy", + "n_films": 67 + }, + { + "name@en": "Romantic Movies", + "n_films": 4 + }, + { + "name@en": "Romantic thriller", + "n_films": 4 + }, + { + "name@en": "Rubble film", + "n_films": 1 + }, + { + "name@en": "Samurai cinema", + "n_films": 39 + }, + { + "name@en": "Satire", + "n_films": 713 + }, + { + "name@en": "School story", + "n_films": 5 + }, + { + "name@en": "Sci-Fi", + "n_films": 3 + }, + { + "name@en": "Sci-Fi Adventure", + "n_films": 31 + }, + { + "name@en": "Sci-Fi Horror", + "n_films": 157 + }, + { + "name@en": "Science", + "n_films": 1 + }, + { + "name@en": "Science and Nature Documentaries", + "n_films": 1 + }, + { + "name@en": "Science fantasy", + "n_films": 2 + }, + { + "name@en": "Science Fiction", + "n_films": 5851 + }, + { + "name@en": "Science fiction film", + "n_films": 20 + }, + { + "name@en": "Science fiction Western", + "n_films": 9 + }, + { + "name@en": "Screwball comedy", + "n_films": 345 + }, + { + "name@en": "Self-help", + "n_films": 13 + }, + { + "name@en": "Semidocumentary", + "n_films": 1 + }, + { + "name@en": "Sepia tone", + "n_films": 1 + }, + { + "name@en": "Serial film", + "n_films": 19 + }, + { + "name@en": "Sermons", + "n_films": 4 + }, + { + "name@en": "Sex comedy", + "n_films": 373 + }, + { + "name@en": "Sex Horror", + "n_films": 8 + }, + { + "name@en": "Sexploitation film", + "n_films": 142 + }, + { + "name@en": "Sh\u014djo manga", + "n_films": 1 + }, + { + "name@en": "Short Film", + "n_films": 46476 + }, + { + "name@en": "Short story", + "n_films": 1 + }, + { + "name@en": "Showbiz Dramas", + "n_films": 1 + }, + { + "name@en": "Silent comedy", + "n_films": 7 + }, + { + "name@en": "Silent film", + "n_films": 6955 + }, + { + "name@en": "Singing cowboy", + "n_films": 5 + }, + { + "name@en": "Sketch comedy", + "n_films": 1 + }, + { + "name@en": "Slapstick", + "n_films": 614 + }, + { + "name@en": "Slasher", + "n_films": 1198 + }, + { + "name@en": "Slice of life", + "n_films": 96 + }, + { + "name@en": "Snuff film", + "n_films": 1 + }, + { + "name@en": "Social & Cultural Documentaries", + "n_films": 5 + }, + { + "name@en": "Social commentary", + "n_films": 1 + }, + { + "name@en": "Social film", + "n_films": 5 + }, + { + "name@en": "Social guidance film", + "n_films": 16 + }, + { + "name@en": "Social issues", + "n_films": 3 + }, + { + "name@en": "Social problem film", + "n_films": 155 + }, + { + "name@en": "Softcore pornography", + "n_films": 246 + }, + { + "name@en": "South Asian cinema", + "n_films": 358 + }, + { + "name@en": "Southeast Asian cinema", + "n_films": 7 + }, + { + "name@en": "Southern Gothic", + "n_films": 1 + }, + { + "name@en": "Space opera", + "n_films": 4 + }, + { + "name@en": "Space Western", + "n_films": 12 + }, + { + "name@en": "Spaghetti Western", + "n_films": 153 + }, + { + "name@en": "Special Interest", + "n_films": 41 + }, + { + "name@en": "Spiritual film", + "n_films": 297 + }, + { + "name@en": "Spirituality", + "n_films": 1 + }, + { + "name@en": "Splatter film", + "n_films": 35 + }, + { + "name@en": "Sponsored film", + "n_films": 9 + }, + { + "name@en": "Sports", + "n_films": 111 + }, + { + "name@en": "Sports films", + "n_films": 6114 + }, + { + "name@en": "Spy fiction", + "n_films": 1 + }, + { + "name@en": "Spy film", + "n_films": 554 + }, + { + "name@en": "Stand-up comedy", + "n_films": 686 + }, + { + "name@en": "Star vehicle", + "n_films": 1 + }, + { + "name@en": "Steampunk", + "n_films": 19 + }, + { + "name@en": "Stoner film", + "n_films": 46 + }, + { + "name@en": "Stop motion", + "n_films": 128 + }, + { + "name@en": "Structural film", + "n_films": 1 + }, + { + "name@en": "Superhero movie", + "n_films": 306 + }, + { + "name@en": "Supermarionation", + "n_films": 2 + }, + { + "name@en": "Supernatural", + "n_films": 633 + }, + { + "name@en": "Supernatural Horror", + "n_films": 4 + }, + { + "name@en": "Surf film", + "n_films": 22 + }, + { + "name@en": "Surrealism", + "n_films": 97 + }, + { + "name@en": "Suspense", + "n_films": 993 + }, + { + "name@en": "Swashbuckler film", + "n_films": 137 + }, + { + "name@en": "Sword and Sandal film", + "n_films": 53 + }, + { + "name@en": "Sword and sorcery", + "n_films": 61 + }, + { + "name@en": "Talk show", + "n_films": 8 + }, + { + "name@en": "Talkie", + "n_films": 2 + }, + { + "name@en": "Tamil cinema", + "n_films": 2817 + }, + { + "name@en": "Tearjerkers", + "n_films": 1 + }, + { + "name@en": "Technicolor", + "n_films": 6 + }, + { + "name@en": "Techno-thriller", + "n_films": 1 + }, + { + "name@en": "Teen drama", + "n_films": 4 + }, + { + "name@en": "Teen film", + "n_films": 1047 + }, + { + "name@en": "Teen pornography", + "n_films": 3 + }, + { + "name@en": "Teen Screams", + "n_films": 1 + }, + { + "name@en": "Teleplay", + "n_films": 2 + }, + { + "name@en": "Television advertisement", + "n_films": 4 + }, + { + "name@en": "Television documentary", + "n_films": 93 + }, + { + "name@en": "Television film", + "n_films": 1414 + }, + { + "name@en": "Television Film", + "n_films": 1 + }, + { + "name@en": "Television pilot", + "n_films": 6 + }, + { + "name@en": "Terrorism", + "n_films": 1 + }, + { + "name@en": "Theatrical Performances", + "n_films": 1 + }, + { + "name@en": "Therimin music", + "n_films": 3 + }, + { + "name@en": "Thrash metal", + "n_films": 1 + }, + { + "name@en": "Thriller", + "n_films": 17716 + }, + { + "name@en": "Time travel", + "n_films": 67 + }, + { + "name@en": "Tokusatsu", + "n_films": 4 + }, + { + "name@en": "Tollywood", + "n_films": 1868 + }, + { + "name@en": "Tragicomedy", + "n_films": 76 + }, + { + "name@en": "Training film", + "n_films": 7 + }, + { + "name@en": "Transforming robots", + "n_films": 1 + }, + { + "name@en": "Transsexualism", + "n_films": 2 + }, + { + "name@en": "Travel", + "n_films": 683 + }, + { + "name@en": "Travel & Adventure Documentaries", + "n_films": 1 + }, + { + "name@en": "Travel documentary", + "n_films": 5 + }, + { + "name@en": "Trial drama", + "n_films": 300 + }, + { + "name@en": "Tribute", + "n_films": 1 + }, + { + "name@en": "Triller", + "n_films": 1 + }, + { + "name@en": "True crime", + "n_films": 60 + }, + { + "name@en": "Tutorial", + "n_films": 1 + }, + { + "name@en": "TV Documentaries", + "n_films": 1 + }, + { + "name@en": "Underground film", + "n_films": 6 + }, + { + "name@en": "Vampire film", + "n_films": 18 + }, + { + "name@en": "Variety show", + "n_films": 1 + }, + { + "name@en": "Vigilante film", + "n_films": 1 + }, + { + "name@en": "Viral video", + "n_films": 1 + }, + { + "name@en": "War", + "n_films": 20 + }, + { + "name@en": "War effort", + "n_films": 1 + }, + { + "name@en": "War film", + "n_films": 5288 + }, + { + "name@en": "Web documentary", + "n_films": 2 + }, + { + "name@en": "Webisode", + "n_films": 2 + }, + { + "name@en": "Werewolf fiction", + "n_films": 4 + }, + { + "name@en": "Western", + "n_films": 4099 + }, + { + "name@en": "Whodunit", + "n_films": 87 + }, + { + "name@en": "Women in prison film", + "n_films": 13 + }, + { + "name@en": "Workplace Comedy", + "n_films": 86 + }, + { + "name@en": "World cinema", + "n_films": 21668 + }, + { + "name@en": "World history", + "n_films": 27 + }, + { + "name@en": "Wrestling", + "n_films": 1 + }, + { + "name@en": "Wuxia", + "n_films": 110 + }, + { + "name@en": "Yakuza film", + "n_films": 22 + }, + { + "name@en": "Youth", + "n_films": 1 + }, + { + "name@en": "Yuri", + "n_films": 2 + }, + { + "name@en": "Z movie", + "n_films": 2 + }, + { + "name@en": "Zapata Western", + "n_films": 1 + }, + { + "name@en": "Zombie", + "n_films": 4 + }, + { + "name@en": "Zombie", + "n_films": 8 + }, + { + "name@en": "Zombie comedy", + "n_films": 3 + }, + { + "name@en": "Zombie Film", + "n_films": 306 + }, + { + "name@en": "Zombie Movies", + "n_films": 2 + }, + { + "n_films": 2 + } + ] +} diff --git a/types/locked_sharded_map.go b/types/locked_sharded_map.go new file mode 100644 index 00000000000..19db5ff8cea --- /dev/null +++ b/types/locked_sharded_map.go @@ -0,0 +1,187 @@ +/* + * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +// Package types contains some very common utilities used by Dgraph. These utilities +// are of "miscellaneous" nature, e.g., error checking. +package types + +import ( + "sync" + + "github.com/dgryski/go-farm" +) + +// LockedShardedMap is a thread-safe, sharded map with generic key-value types. +type LockedShardedMap[K comparable, V any] struct { + shards []map[K]V + locks []sync.RWMutex +} + +// NewLockedShardedMap creates a new LockedShardedMap. +func NewLockedShardedMap[K comparable, V any]() *LockedShardedMap[K, V] { + shards := make([]map[K]V, NumShards) + locks := make([]sync.RWMutex, NumShards) + for i := range shards { + shards[i] = make(map[K]V) + } + return &LockedShardedMap[K, V]{shards: shards, locks: locks} +} + +func (s *LockedShardedMap[K, V]) getShardIndex(key K) int { + // Only works for integer-like keys (uint64 etc). For generic types, + // a better hash function is needed. We'll assume uint64 for now. + switch k := any(key).(type) { + case uint64: + return int(k % uint64(NumShards)) + case string: + return int(farm.Fingerprint64([]byte(k)) % uint64(NumShards)) + default: + panic("LockedShardedMap only supports uint64 and string keys for now") + } +} + +func (s *LockedShardedMap[K, V]) Set(key K, value V) { + if s == nil { + return + } + index := s.getShardIndex(key) + s.locks[index].Lock() + defer s.locks[index].Unlock() + s.shards[index][key] = value +} + +func (s *LockedShardedMap[K, V]) Get(key K) (V, bool) { + var zero V + if s == nil { + return zero, false + } + index := s.getShardIndex(key) + s.locks[index].RLock() + defer s.locks[index].RUnlock() + val, ok := s.shards[index][key] + return val, ok +} + +func (s *LockedShardedMap[K, V]) Update(key K, update func(V, bool) V) { + if s == nil { + return + } + index := s.getShardIndex(key) + s.locks[index].Lock() + defer s.locks[index].Unlock() + val, ok := s.shards[index][key] + s.shards[index][key] = update(val, ok) +} + +func (s *LockedShardedMap[K, V]) Merge(other *LockedShardedMap[K, V], ag func(a, b V) V) { + var wg sync.WaitGroup + for i := range s.shards { + wg.Add(1) + go func(i int) { + defer wg.Done() + otherShard := other.shards[i] + for k, v := range otherShard { + s.locks[i].Lock() + if existing, ok := s.shards[i][k]; ok { + s.shards[i][k] = ag(existing, v) + } else { + s.shards[i][k] = v + } + s.locks[i].Unlock() + } + }(i) + } + wg.Wait() +} + +func (s *LockedShardedMap[K, V]) Len() int { + if s == nil { + return 0 + } + var count int + for i := range s.shards { + s.locks[i].RLock() + count += len(s.shards[i]) + s.locks[i].RUnlock() + } + return count +} + +func (s *LockedShardedMap[K, V]) ParallelIterate(f func(K, V) error) error { + if s == nil { + return nil + } + + var ( + wg sync.WaitGroup + errCh = make(chan error, 1) + once sync.Once + ) + + for i := range s.shards { + wg.Add(1) + go func(i int) { + defer wg.Done() + + s.locks[i].RLock() + defer s.locks[i].RUnlock() + + for k, v := range s.shards[i] { + if err := f(k, v); err != nil { + once.Do(func() { + errCh <- err + }) + return + } + } + }(i) + } + + // Wait in a separate goroutine so we can still select on errCh. + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + + select { + case err := <-errCh: + return err + case <-done: + return nil + } +} + +func (s *LockedShardedMap[K, V]) Iterate(f func(K, V) error) error { + if s == nil { + return nil + } + for i := range s.shards { + s.locks[i].RLock() + for k, v := range s.shards[i] { + if err := f(k, v); err != nil { + s.locks[i].RUnlock() + return err + } + } + s.locks[i].RUnlock() + } + return nil +} + +func (s *LockedShardedMap[K, V]) IsEmpty() bool { + if s == nil { + return true + } + for i := range s.shards { + s.locks[i].RLock() + if len(s.shards[i]) > 0 { + s.locks[i].RUnlock() + return false + } + s.locks[i].RUnlock() + } + return true +} diff --git a/worker/draft.go b/worker/draft.go index 124f59057a5..776840aef5d 100644 --- a/worker/draft.go +++ b/worker/draft.go @@ -516,6 +516,21 @@ func (n *node) applyMutations(ctx context.Context, proposal *pb.Proposal) (rerr m := proposal.Mutations + txn := posting.Oracle().RegisterStartTs(m.StartTs) + if txn.ShouldAbort() { + span.AddEvent("Txn should abort.", trace.WithAttributes( + attribute.Int64("start_ts", int64(m.StartTs)), + )) + return x.ErrConflict + } + // Discard the posting lists from cache to release memory at the end. + defer txn.Update() + + if t := x.WorkerConfig.MutationsPipelineThreshold; t > 0 && len(m.Edges) >= t { + mp := posting.NewMutationPipeline(txn) + return mp.Process(ctx, m.Edges) + } + // It is possible that the user gives us multiple versions of the same edge, one with no facets // and another with facets. In that case, use stable sort to maintain the ordering given to us // by the user. @@ -528,16 +543,6 @@ func (n *node) applyMutations(ctx context.Context, proposal *pb.Proposal) (rerr return ei.GetEntity() < ej.GetEntity() }) - txn := posting.Oracle().RegisterStartTs(m.StartTs) - if txn.ShouldAbort() { - span.AddEvent("Txn should abort.", trace.WithAttributes( - attribute.Int64("start_ts", int64(m.StartTs)), - )) - return x.ErrConflict - } - // Discard the posting lists from cache to release memory at the end. - defer txn.Update() - process := func(edges []*pb.DirectedEdge) error { var retries int for _, edge := range edges { diff --git a/worker/mutation.go b/worker/mutation.go index 5efc6b2d1d7..fc52f63ec78 100644 --- a/worker/mutation.go +++ b/worker/mutation.go @@ -54,6 +54,16 @@ func isDeletePredicateEdge(edge *pb.DirectedEdge) bool { return edge.Entity == 0 && isStarAll(edge.Value) } +func newRunMutations(ctx context.Context, edges []*pb.DirectedEdge, txn *posting.Txn) error { + mp := posting.NewMutationPipeline(txn) + return mp.Process(ctx, edges) +} + +func newRunMutation(ctx context.Context, edge *pb.DirectedEdge, txn *posting.Txn) error { + mp := posting.NewMutationPipeline(txn) + return mp.Process(ctx, []*pb.DirectedEdge{edge}) +} + // runMutation goes through all the edges and applies them. func runMutation(ctx context.Context, edge *pb.DirectedEdge, txn *posting.Txn) error { ctx = schema.GetWriteContext(ctx) diff --git a/worker/mutation_unit_test.go b/worker/mutation_unit_test.go index 062b5152d2a..8126e40f701 100644 --- a/worker/mutation_unit_test.go +++ b/worker/mutation_unit_test.go @@ -47,8 +47,8 @@ func TestReverseEdge(t *testing.T) { Op: pb.DirectedEdge_DEL, } - x.Check(runMutation(ctx, edge, txn)) - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) pl, err := txn.Get(x.DataKey(attr, 1)) require.NoError(t, err) @@ -105,10 +105,10 @@ func TestReverseEdgeSetDel(t *testing.T) { Op: pb.DirectedEdge_SET, } - x.Check(runMutation(ctx, edgeSet1, txn)) - x.Check(runMutation(ctx, edgeSet2, txn)) - x.Check(runMutation(ctx, edgeSet3, txn)) - x.Check(runMutation(ctx, edgeDel, txn)) + x.Check(newRunMutation(ctx, edgeSet1, txn)) + x.Check(newRunMutation(ctx, edgeSet2, txn)) + x.Check(newRunMutation(ctx, edgeSet3, txn)) + x.Check(newRunMutation(ctx, edgeDel, txn)) pl, err := txn.Get(x.ReverseKey(attr, 2)) require.NoError(t, err) diff --git a/worker/pipeline_bench_test.go b/worker/pipeline_bench_test.go new file mode 100644 index 00000000000..e2d68032d47 --- /dev/null +++ b/worker/pipeline_bench_test.go @@ -0,0 +1,210 @@ +/* + * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package worker + +import ( + "context" + "fmt" + "os" + "testing" + + "github.com/dgraph-io/badger/v4" + "github.com/dgraph-io/dgraph/v25/posting" + "github.com/dgraph-io/dgraph/v25/protos/pb" + "github.com/dgraph-io/dgraph/v25/schema" + "github.com/dgraph-io/dgraph/v25/x" +) + +// Benchmarks comparing the legacy serial mutation path (runMutation per edge) +// with the new per-predicate mutation pipeline (newRunMutations). +// +// What the pipeline ought to win on: +// - many predicates per transaction → one goroutine per predicate +// - many indexed edges per predicate → 10-way intra-predicate +// parallelism on tokenization +// +// What it shouldn't help (and may regret): +// - tiny mutations (1-2 edges, 1 predicate) where goroutine spin-up cost +// dominates the mutation work +// +// Each iteration is a single transaction: build a fresh batch of edges, +// run mutations, txn.Update(), CommitToDisk. We do NOT include the b.ResetTimer() +// before edge construction because edge construction is part of the +// per-transaction cost the pipeline is supposed to amortize. + +func benchSetup(b *testing.B, schemaTxt string) *badger.DB { + b.Helper() + dir, err := os.MkdirTemp("", "pipeline_bench_") + if err != nil { + b.Fatal(err) + } + b.Cleanup(func() { _ = os.RemoveAll(dir) }) + + ps, err := badger.OpenManaged(badger.DefaultOptions(dir).WithLoggingLevel(badger.ERROR)) + if err != nil { + b.Fatal(err) + } + b.Cleanup(func() { _ = ps.Close() }) + + posting.Init(ps, 0, false) + Init(ps) + posting.Oracle().ResetTxns() + if err := schema.ParseBytes([]byte(schemaTxt), 1); err != nil { + b.Fatal(err) + } + return ps +} + +// buildEdges constructs numPreds*edgesPerPred edges across distinct predicates, +// indexed-string-valued. The same generator drives both legacy and pipeline +// runs so the input is identical. +func buildEdges(numPreds, edgesPerPred int, baseUid uint64) []*pb.DirectedEdge { + edges := make([]*pb.DirectedEdge, 0, numPreds*edgesPerPred) + for p := 0; p < numPreds; p++ { + attr := x.AttrInRootNamespace(fmt.Sprintf("p%d", p)) + for e := 0; e < edgesPerPred; e++ { + edges = append(edges, &pb.DirectedEdge{ + Entity: baseUid + uint64(e), + Attr: attr, + Value: []byte(fmt.Sprintf("v%d_%d", p, e)), + ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + }) + } + } + return edges +} + +// schemaForPreds emits "p0: string @index(exact) ., p1: ..., ..." (or no +// index, depending on indexed). Each predicate is a distinct list-or-scalar. +func schemaForPreds(numPreds int, indexed bool, list bool) string { + var b []byte + for p := 0; p < numPreds; p++ { + ty := "string" + if list { + ty = "[string]" + } + idx := "" + if indexed { + idx = " @index(exact)" + } + b = append(b, []byte(fmt.Sprintf("p%d: %s%s .\n", p, ty, idx))...) + } + return string(b) +} + +// runOne executes one transaction's mutations through the chosen path. +// startTs/commitTs must be unique per call. +func runOnePipeline(b *testing.B, ps *badger.DB, edges []*pb.DirectedEdge, startTs, commitTs uint64) { + b.Helper() + txn := posting.Oracle().RegisterStartTs(startTs) + if err := newRunMutations(context.Background(), edges, txn); err != nil { + b.Fatal(err) + } + txn.Update() + w := posting.NewTxnWriter(ps) + if err := txn.CommitToDisk(w, commitTs); err != nil { + b.Fatal(err) + } + if err := w.Flush(); err != nil { + b.Fatal(err) + } + txn.UpdateCachedKeys(commitTs) +} + +func runOneLegacy(b *testing.B, ps *badger.DB, edges []*pb.DirectedEdge, startTs, commitTs uint64) { + b.Helper() + txn := posting.Oracle().RegisterStartTs(startTs) + for _, e := range edges { + if err := runMutation(context.Background(), e, txn); err != nil { + b.Fatal(err) + } + } + txn.Update() + w := posting.NewTxnWriter(ps) + if err := txn.CommitToDisk(w, commitTs); err != nil { + b.Fatal(err) + } + if err := w.Flush(); err != nil { + b.Fatal(err) + } + txn.UpdateCachedKeys(commitTs) +} + +// runBench runs sub-benchmarks (legacy vs pipeline) for a single +// (numPreds, edgesPerPred, indexed, list) configuration. +func runBench(b *testing.B, numPreds, edgesPerPred int, indexed, list bool) { + for _, mode := range []struct { + name string + fn func(*testing.B, *badger.DB, []*pb.DirectedEdge, uint64, uint64) + }{ + {"legacy", runOneLegacy}, + {"pipeline", runOnePipeline}, + } { + b.Run(mode.name, func(b *testing.B) { + ps := benchSetup(b, schemaForPreds(numPreds, indexed, list)) + b.ReportAllocs() + b.ResetTimer() + ts := uint64(10) + for i := 0; i < b.N; i++ { + edges := buildEdges(numPreds, edgesPerPred, uint64(i)*1_000_000+1) + mode.fn(b, ps, edges, ts, ts+1) + ts += 2 + } + }) + } +} + +// 1 predicate, 1 edge — smallest possible mutation. Pipeline overhead +// is most visible here. +func BenchmarkMutate_1pred_1edge_indexed(b *testing.B) { + runBench(b, 1, 1, true, false) +} + +// 1 predicate, 100 indexed edges — exercises intra-predicate +// tokenization parallelism. +func BenchmarkMutate_1pred_100edges_indexed(b *testing.B) { + runBench(b, 1, 100, true, false) +} + +// 10 predicates, 1 edge each — per-predicate parallelism with light work +// per predicate. +func BenchmarkMutate_10preds_1edge_indexed(b *testing.B) { + runBench(b, 10, 1, true, false) +} + +// 10 predicates, 100 edges each — full benefit case: per-predicate AND +// intra-predicate parallelism on indexed work. +func BenchmarkMutate_10preds_100edges_indexed(b *testing.B) { + runBench(b, 10, 100, true, false) +} + +// 1 predicate, 1000 indexed edges — heavy intra-predicate. +func BenchmarkMutate_1pred_1000edges_indexed(b *testing.B) { + runBench(b, 1, 1000, true, false) +} + +// 10 predicates, 1000 edges each — large mutation, indexed. +func BenchmarkMutate_10preds_1000edges_indexed(b *testing.B) { + runBench(b, 10, 1000, true, false) +} + +// Non-indexed counterparts isolate per-predicate parallelism from the +// tokenization parallelism. +func BenchmarkMutate_10preds_1000edges_noindex(b *testing.B) { + runBench(b, 10, 1000, false, false) +} + +// Very large indexed mutation: 50 predicates × 1000 edges each = 50k edges. +// Where the pipeline should shine most. +func BenchmarkMutate_50preds_1000edges_indexed(b *testing.B) { + runBench(b, 50, 1000, true, false) +} + +// 50 predicates, 100 edges each (5k edges) — typical-ish bulk write shape. +func BenchmarkMutate_50preds_100edges_indexed(b *testing.B) { + runBench(b, 50, 100, true, false) +} diff --git a/worker/server_state.go b/worker/server_state.go index d9905132442..071f163869d 100644 --- a/worker/server_state.go +++ b/worker/server_state.go @@ -42,7 +42,8 @@ const ( GraphQLDefaults = `introspection=true; debug=false; extensions=true; poll-interval=1s; ` + `lambda-url=;` CacheDefaults = `size-mb=4096; percentage=40,40,20; remove-on-update=false` - FeatureFlagsDefaults = `normalize-compatibility-mode=; enable-detailed-metrics=false; log-slow-query-threshold=0` + FeatureFlagsDefaults = `normalize-compatibility-mode=; enable-detailed-metrics=false; ` + + `log-slow-query-threshold=0; mutations-pipeline-threshold=1` ) // ServerState holds the state of the Dgraph server. diff --git a/worker/sort_test.go b/worker/sort_test.go index def4e7a9d02..3bf624a3a81 100644 --- a/worker/sort_test.go +++ b/worker/sort_test.go @@ -8,16 +8,24 @@ package worker import ( "context" "fmt" + "math" "math/rand" "os" + "strconv" + "sync" + "sync/atomic" "testing" "github.com/dgraph-io/badger/v4" bpb "github.com/dgraph-io/badger/v4/pb" + "github.com/dgraph-io/dgo/v250/protos/api" "github.com/dgraph-io/dgraph/v25/posting" "github.com/dgraph-io/dgraph/v25/protos/pb" "github.com/dgraph-io/dgraph/v25/schema" + "github.com/dgraph-io/dgraph/v25/tok" + "github.com/dgraph-io/dgraph/v25/types" "github.com/dgraph-io/dgraph/v25/x" + "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -90,6 +98,952 @@ func TestEmptyTypeSchema(t *testing.T) { x.ParseNamespaceAttr(types[0].TypeName) } +func TestDatetime(t *testing.T) { + // Setup temporary directory for Badger DB + dir, err := os.MkdirTemp("", "storetest_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + opt := badger.DefaultOptions(dir) + ps, err := badger.OpenManaged(opt) + require.NoError(t, err) + posting.Init(ps, 0, false) + Init(ps) + + // Set schema + schemaTxt := ` + t: datetime @index(year) . + ` + err = schema.ParseBytes([]byte(schemaTxt), 1) + require.NoError(t, err) + + ctx := context.Background() + newRunMutation := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { + txn := posting.Oracle().RegisterStartTs(startTs) + for _, edge := range edges { + require.NoError(t, newRunMutation(ctx, edge, txn)) + } + txn.Update() + writer := posting.NewTxnWriter(ps) + require.NoError(t, txn.CommitToDisk(writer, commitTs)) + require.NoError(t, writer.Flush()) + txn.UpdateCachedKeys(commitTs) + } + + newRunMutation(1, 3, []*pb.DirectedEdge{ + { + Entity: 1, + Attr: x.AttrInRootNamespace("t"), + Value: []byte("2020-01-01T00:00:00Z"), + ValueType: pb.Posting_DEFAULT, + Op: pb.DirectedEdge_SET, + }, + }) + +} + +type indexMutationInfo struct { + tokenizers []tok.Tokenizer + factorySpecs []*tok.FactoryCreateSpec + edge *pb.DirectedEdge // Represents the original uid -> value edge. + val types.Val + op pb.DirectedEdge_Op +} + +func indexTokens(ctx context.Context, info *indexMutationInfo) ([]string, error) { + attr := info.edge.Attr + lang := info.edge.GetLang() + + schemaType, err := schema.State().TypeOf(attr) + if err != nil || !schemaType.IsScalar() { + return nil, errors.Errorf("Cannot index attribute %s of type object.", attr) + } + + if !schema.State().IsIndexed(ctx, attr) { + return nil, errors.Errorf("Attribute %s is not indexed.", attr) + } + sv, err := types.Convert(info.val, schemaType) + if err != nil { + return nil, errors.Wrap(err, "Cannot convert value to scalar type") + } + + var tokens []string + for _, it := range info.tokenizers { + toks, err := tok.BuildTokens(sv.Value, tok.GetTokenizerForLang(it, lang)) + if err != nil { + return tokens, errors.Wrapf(err, "Cannot build tokens for attribute %s", attr) + } + tokens = append(tokens, toks...) + } + return tokens, nil +} + +func TestStringIndexWithLang(t *testing.T) { + // Setup temporary directory for Badger DB + dir, err := os.MkdirTemp("", "storetest_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + opt := badger.DefaultOptions(dir) + ps, err := badger.OpenManaged(opt) + require.NoError(t, err) + posting.Init(ps, 0, false) + Init(ps) + + // Set schema + schemaTxt := ` + name: string @index(fulltext, trigram, term, exact) @lang . + ` + + err = schema.ParseBytes([]byte(schemaTxt), 1) + require.NoError(t, err) + + ctx := context.Background() + newRunMutation := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { + txn := posting.Oracle().RegisterStartTs(startTs) + require.NoError(t, newRunMutations(ctx, edges, txn)) + txn.Update() + writer := posting.NewTxnWriter(ps) + require.NoError(t, txn.CommitToDisk(writer, commitTs)) + require.NoError(t, writer.Flush()) + txn.UpdateCachedKeys(commitTs) + } + + attr := x.AttrInRootNamespace("name") + + // Prepare 400 mutations across 4 threads, 100 per thread (kept modest for stability). + const ( + threads = 10 + perThread = 20000 + total = threads * perThread + baseStartTs = uint64(10) + ) + + // uid -> value map + values := make(map[uint64]string, total) + for i := 0; i < total; i++ { + uid := uint64(i + 1) + // Simple deterministic values with shared tokens and unique numbers. + values[uid] = fmt.Sprintf("title %d", i+1) + } + + // Build expected token -> set of uids + tokenizers := schema.State().Tokenizer(ctx, attr) + expected := make(map[string]map[uint64]struct{}, total) + for uid, val := range values { + info := &indexMutationInfo{ + tokenizers: tokenizers, + op: pb.DirectedEdge_SET, + val: types.Val{Tid: types.StringID, Value: []byte(val)}, + edge: &pb.DirectedEdge{ + Attr: attr, + Value: []byte(val), + Lang: "en", + Op: pb.DirectedEdge_SET, + }, + } + toks, err := indexTokens(ctx, info) + require.NoError(t, err) + for _, tk := range toks { + if expected[tk] == nil { + expected[tk] = make(map[uint64]struct{}) + } + expected[tk][uid] = struct{}{} + } + } + + // Run 4 threads; each thread writes 100 edges with distinct ts + var wg sync.WaitGroup + wg.Add(threads) + for th := 0; th < threads; th++ { + th := th + go func() { + defer wg.Done() + start := th*perThread + 1 + end := start + perThread + edges := make([]*pb.DirectedEdge, 0, perThread) + for i := start; i < end; i++ { + uid := uint64(i) + edges = append(edges, &pb.DirectedEdge{ + Entity: uid, + Attr: attr, + Value: []byte(values[uid]), + ValueType: pb.Posting_DEFAULT, + Lang: "en", + Op: pb.DirectedEdge_SET, + }) + } + sTs := baseStartTs + uint64(th*10) + cTs := sTs + 2 + newRunMutation(sTs, cTs, edges) + }() + } + wg.Wait() + + // Verify all tokens have the expected UIDs. + readTs := baseStartTs + uint64(threads*10) + 10 + for tk, uidset := range expected { + key := x.IndexKey(attr, tk) + txn := posting.Oracle().RegisterStartTs(readTs) + pl, err := txn.Get(key) + require.NoError(t, err) + lst, err := pl.Uids(posting.ListOptions{ReadTs: readTs}) + require.NoError(t, err) + got := make(map[uint64]struct{}, len(lst.Uids)) + for _, u := range lst.Uids { + got[u] = struct{}{} + } + // Compare sets + require.Equal(t, len(uidset), len(got), "mismatch uid count for token %q", tk) + for u := range uidset { + if _, ok := got[u]; !ok { + t.Fatalf("missing uid %d for token %q", u, tk) + } + } + } +} + +func TestCount(t *testing.T) { + t.Skip("Inherently racy: bypasses the Oracle conflict-checking commit path. " + + "Legacy and new pipeline both fail. Re-enable when the harness uses real txn conflicts.") + // Setup temporary directory for Badger DB + dir, err := os.MkdirTemp("", "storetest_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + opt := badger.DefaultOptions(dir) + ps, err := badger.OpenManaged(opt) + require.NoError(t, err) + posting.Init(ps, 0, false) + Init(ps) + + // Set schema + schemaTxt := ` + friends: [uid] @count . + ` + + err = schema.ParseBytes([]byte(schemaTxt), 1) + require.NoError(t, err) + ctx := context.Background() + newRunMutation := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { + txn := posting.Oracle().RegisterStartTs(startTs) + require.NoError(t, newRunMutations(ctx, edges, txn)) + txn.Update() + writer := posting.NewTxnWriter(ps) + require.NoError(t, txn.CommitToDisk(writer, commitTs)) + require.NoError(t, writer.Flush()) + txn.UpdateCachedKeys(commitTs) + } + + pred := x.AttrInRootNamespace("friends") + + // Prepare mutations such that each subject gets multiple uid edges, and + // each edge is added from a different thread. We also send multiple + // batches per thread. + const ( + subjects = 10 // total number of subjects/entities + edgesPer = 5 // number of edges per subject + threads = 2 // one thread per edge ordinal, touching all subjects + baseStartTs = uint64(10) + total = subjects * edgesPer + ) + + // 1) Pre-generate all mutations into one big slice + edgesAll := make([]*pb.DirectedEdge, 0, total) + for subj := 1; subj <= subjects; subj++ { + uid := uint64(subj) + for e := 0; e < edgesPer; e++ { + // Unique object per (subject, edge-ordinal) pair to avoid duplicates. + // Ensures exactly 'edgesPer' distinct UIDs per subject. + obj := uint64(1_000_000 + subj*100 + e) + edgesAll = append(edgesAll, &pb.DirectedEdge{ + Entity: uid, + Attr: pred, + ValueId: obj, + ValueType: pb.Posting_UID, + Op: pb.DirectedEdge_SET, + }) + } + } + + // Shuffle the edges to simulate randomness (determinism depends on rand.Seed in package scope) + for i := range edgesAll { + j := rand.Intn(i + 1) + edgesAll[i], edgesAll[j] = edgesAll[j], edgesAll[i] + } + + // 2) Dispatch pre-generated mutations into threads, in multiple batches per thread + var wg sync.WaitGroup + wg.Add(threads) + for th := 0; th < threads; th++ { + th := th + go func() { + defer wg.Done() + // Split each thread's disjoint chunk into multiple batches/transactions + const batches = 5 + chunk := total / threads + chunkStart := th * chunk + chunkEnd := chunkStart + chunk + perBatch := chunk / batches + for b := 0; b < batches; b++ { + batchStart := chunkStart + b*perBatch + batchEnd := batchStart + perBatch + if b == batches-1 { + batchEnd = chunkEnd + } + batch := edgesAll[batchStart:batchEnd] + // Space out start/commit timestamps per thread and per batch to avoid collisions + sTs := baseStartTs + uint64(th*100) + uint64(b*2) + cTs := sTs + 1 + newRunMutation(sTs, cTs, batch) + } + }() + } + wg.Wait() + + // Verify the @count index for the exact number of edges per subject. + countKey := x.CountKey(pred, edgesPer, false) + txn := posting.Oracle().RegisterStartTs(math.MaxUint64) + pl, err := txn.Get(countKey) + require.NoError(t, err) + uids, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + fmt.Println(uids.Uids) + require.Equal(t, subjects, len(uids.Uids)) +} + +// fakeOracle is an in-memory stand-in for the zero Oracle. It hands out +// monotonically increasing timestamps and rejects commits whose conflict +// keys overlap a higher commitTs — same algorithm as +// dgraph/cmd/zero/oracle.go's hasConflict. +type fakeOracle struct { + mu sync.Mutex + nextTs uint64 + keyCommit map[uint64]uint64 // conflict-key fingerprint -> commitTs + committed atomic.Int64 + aborted atomic.Int64 +} + +func newFakeOracle(initialTs uint64) *fakeOracle { + return &fakeOracle{nextTs: initialTs, keyCommit: map[uint64]uint64{}} +} + +func (o *fakeOracle) reserveStartTs() uint64 { + o.mu.Lock() + defer o.mu.Unlock() + o.nextTs++ + return o.nextTs +} + +// tryCommit mirrors zero/oracle.go: for each conflict key, if a later +// commitTs already exists, abort. Else stamp all keys with a fresh +// commitTs and return it. +func (o *fakeOracle) tryCommit(startTs uint64, conflictKeys []uint64) (uint64, bool) { + o.mu.Lock() + defer o.mu.Unlock() + for _, k := range conflictKeys { + if last, ok := o.keyCommit[k]; ok && last > startTs { + o.aborted.Add(1) + return 0, false + } + } + o.nextTs++ + commitTs := o.nextTs + for _, k := range conflictKeys { + o.keyCommit[k] = commitTs + } + o.committed.Add(1) + return commitTs, true +} + +// runPipelineTxn drives a single mutation through the new pipeline with +// real conflict-aware commit semantics. Returns (committed, error). +func runPipelineTxn(t *testing.T, ps *badger.DB, oracle *fakeOracle, + edges []*pb.DirectedEdge) bool { + t.Helper() + startTs := oracle.reserveStartTs() + txn := posting.Oracle().RegisterStartTs(startTs) + + if err := newRunMutations(context.Background(), edges, txn); err != nil { + t.Fatalf("pipeline failed at startTs=%d: %v", startTs, err) + } + + // FillContext bridges plists -> deltas (via Update) and emits the + // txn's conflict keys as base-36 strings on ctx.Keys. + ctxApi := &api.TxnContext{} + txn.FillContext(ctxApi, 1, false) + + keys := make([]uint64, 0, len(ctxApi.Keys)) + for _, k := range ctxApi.Keys { + ki, err := strconv.ParseUint(k, 36, 64) + require.NoError(t, err) + keys = append(keys, ki) + } + + commitTs, ok := oracle.tryCommit(startTs, keys) + if !ok { + return false + } + writer := posting.NewTxnWriter(ps) + require.NoError(t, txn.CommitToDisk(writer, commitTs)) + require.NoError(t, writer.Flush()) + txn.UpdateCachedKeys(commitTs) + return true +} + +// TestPipelineCountIndexConcurrent mirrors the systest's +// TestCountIndexConcurrentSetDelScalarPredicate at unit-test scope: many +// concurrent transactions setting <0x1> "name" against a +// scalar string predicate with @index(exact) @count, with real +// conflict-checking commit semantics. After everything settles, the data +// list for 0x1 should hold exactly one value, the count(1) bucket should +// reference exactly 0x1, and no other count bucket should reference 0x1. +func TestPipelineCountIndexConcurrent(t *testing.T) { + dir, err := os.MkdirTemp("", "storetest_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + ps, err := badger.OpenManaged(badger.DefaultOptions(dir)) + require.NoError(t, err) + defer ps.Close() + posting.Init(ps, 0, false) + Init(ps) + posting.Oracle().ResetTxns() + + require.NoError(t, schema.ParseBytes( + []byte(`name: string @index(exact) @count .`), 1)) + + pred := x.AttrInRootNamespace("name") + const target uint64 = 1 + + oracle := newFakeOracle(10) + + const ( + numRoutines = 10 + txnsPerRoute = 20 + ) + + var wg sync.WaitGroup + wg.Add(numRoutines) + for r := 0; r < numRoutines; r++ { + go func(seed int) { + defer wg.Done() + rnd := rand.New(rand.NewSource(int64(seed))) + for i := 0; i < txnsPerRoute; i++ { + value := []byte(fmt.Sprintf("name%d", rnd.Intn(10000))) + // Retry on conflict — same as a client doing dg.NewTxn().Mutate(). + // Each attempt uses a fresh edge: makePostingFromEdge mutates + // edge.ValueId during processing, and reusing the object across + // attempts would make ValidateAndConvert see it as a uid edge. + // Real production gets a freshly-deserialized edge per Raft apply. + for attempt := 0; attempt < 100; attempt++ { + edge := &pb.DirectedEdge{ + Entity: target, + Attr: pred, + Value: value, + ValueType: pb.Posting_STRING, + Op: pb.DirectedEdge_SET, + } + if runPipelineTxn(t, ps, oracle, []*pb.DirectedEdge{edge}) { + break + } + } + } + }(r) + } + wg.Wait() + + t.Logf("committed=%d aborted=%d", oracle.committed.Load(), oracle.aborted.Load()) + + // Verify final state: exactly one value on 0x1, that uid in count(1) only. + readTxn := posting.Oracle().RegisterStartTs(math.MaxUint64) + + dataKey := x.DataKey(pred, target) + dpl, err := readTxn.Get(dataKey) + require.NoError(t, err) + // Scalar string predicate: AllValues returns the live posting list values + // (one entry for a non-list scalar with a current value). + vals, err := dpl.AllValues(math.MaxUint64) + require.NoError(t, err) + require.Equal(t, 1, len(vals), + "scalar predicate should retain exactly one value, got %v", vals) + + for c := 0; c <= 5; c++ { + ck := x.CountKey(pred, uint32(c), false) + cpl, err := readTxn.Get(ck) + require.NoError(t, err) + cuids, err := cpl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + switch c { + case 1: + require.Equal(t, []uint64{target}, cuids.Uids, + "count(1) bucket must contain exactly the target uid") + default: + require.NotContains(t, cuids.Uids, target, + "count(%d) bucket must not contain the target uid", c) + } + } +} + +// TestPipelineReverseListCount mirrors the [uid] @reverse @count shape from +// the 21million live-load test (genre predicate). One subject points at +// multiple objects in a single transaction; we then verify that BOTH the +// forward data list and the reverse data list are complete. +// +// Background: systest/21million/live's TestQueries/Run_queries/query-017 +// fails consistently with one specific film's `genre = Animation` edge +// missing, while other genre edges from the same film are intact. This +// is the smallest in-process repro of the same forward/reverse fanout +// pattern that the live loader hits. +func TestPipelineReverseListCount(t *testing.T) { + dir, err := os.MkdirTemp("", "pipelinerevcount_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + ps, err := badger.OpenManaged(badger.DefaultOptions(dir)) + require.NoError(t, err) + defer ps.Close() + posting.Init(ps, 0, false) + Init(ps) + posting.Oracle().ResetTxns() + + require.NoError(t, schema.ParseBytes( + []byte(`genre: [uid] @reverse @count .`), 1)) + + pred := x.AttrInRootNamespace("genre") + + // Subject -> list of object uids. Mirrors a film with multiple genres, + // and several films sharing some genres. + const ( + madagascar = uint64(100) + brotherly = uint64(101) + animation = uint64(200) + shortFilm = uint64(201) + comedy = uint64(202) + ) + wantForward := map[uint64][]uint64{ + madagascar: {animation, shortFilm}, + brotherly: {animation, shortFilm, comedy}, + } + + // Single transaction containing all edges (the simplest case — no + // concurrency, no batching, no Oracle conflict path). If this fails, + // the bug is purely in the forward+reverse fanout within ProcessList. + edges := []*pb.DirectedEdge{} + for subj, objs := range wantForward { + for _, obj := range objs { + edges = append(edges, &pb.DirectedEdge{ + Entity: subj, + Attr: pred, + ValueId: obj, + ValueType: pb.Posting_UID, + Op: pb.DirectedEdge_SET, + }) + } + } + + txn := posting.Oracle().RegisterStartTs(10) + require.NoError(t, newRunMutations(context.Background(), edges, txn)) + txn.Update() + w := posting.NewTxnWriter(ps) + require.NoError(t, txn.CommitToDisk(w, 11)) + require.NoError(t, w.Flush()) + txn.UpdateCachedKeys(11) + + readTxn := posting.Oracle().RegisterStartTs(math.MaxUint64) + + // Forward: each subject must have all its expected objects. + for subj, wantObjs := range wantForward { + key := x.DataKey(pred, subj) + pl, err := readTxn.Get(key) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantObjs, got.Uids, + "forward list for subject %d", subj) + } + + // Reverse: each object must have all the subjects that point at it. + wantReverse := map[uint64][]uint64{} + for subj, objs := range wantForward { + for _, obj := range objs { + wantReverse[obj] = append(wantReverse[obj], subj) + } + } + for obj, wantSubjs := range wantReverse { + key := x.ReverseKey(pred, obj) + pl, err := readTxn.Get(key) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantSubjs, got.Uids, + "reverse list for object %d", obj) + } +} + +// TestPipelineReverseListCountMultiBatch escalates TestPipelineReverseListCount +// by spreading edges across many sequential transactions, similar to how the +// live loader chunks edges into many batches. Each batch is one transaction. +func TestPipelineReverseListCountMultiBatch(t *testing.T) { + dir, err := os.MkdirTemp("", "pipelinerevcount2_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + ps, err := badger.OpenManaged(badger.DefaultOptions(dir)) + require.NoError(t, err) + defer ps.Close() + posting.Init(ps, 0, false) + Init(ps) + posting.Oracle().ResetTxns() + + require.NoError(t, schema.ParseBytes( + []byte(`genre: [uid] @reverse @count .`), 1)) + + pred := x.AttrInRootNamespace("genre") + + // Many subjects, many objects, every (subject, object) combination — a + // dense forward fanout that pushes every reverse list to grow as well. + const ( + nSubjects = 50 + nObjects = 20 + ) + subjBase := uint64(10000) + objBase := uint64(20000) + + // Build every edge. + allEdges := make([]*pb.DirectedEdge, 0, nSubjects*nObjects) + for s := 0; s < nSubjects; s++ { + for o := 0; o < nObjects; o++ { + allEdges = append(allEdges, &pb.DirectedEdge{ + Entity: subjBase + uint64(s), + Attr: pred, + ValueId: objBase + uint64(o), + ValueType: pb.Posting_UID, + Op: pb.DirectedEdge_SET, + }) + } + } + + // Split into batches of 7 — a value chosen to make each batch carry a + // non-trivial mix of subjects and objects per ProcessList run. + const batchSize = 7 + ts := uint64(10) + for start := 0; start < len(allEdges); start += batchSize { + end := start + batchSize + if end > len(allEdges) { + end = len(allEdges) + } + batch := allEdges[start:end] + + txn := posting.Oracle().RegisterStartTs(ts) + require.NoError(t, newRunMutations(context.Background(), batch, txn)) + txn.Update() + w := posting.NewTxnWriter(ps) + require.NoError(t, txn.CommitToDisk(w, ts+1)) + require.NoError(t, w.Flush()) + txn.UpdateCachedKeys(ts + 1) + ts += 2 + } + + readTxn := posting.Oracle().RegisterStartTs(math.MaxUint64) + + // Forward: each subject must hold exactly all nObjects objects. + wantForwardObjs := make([]uint64, nObjects) + for o := 0; o < nObjects; o++ { + wantForwardObjs[o] = objBase + uint64(o) + } + for s := 0; s < nSubjects; s++ { + subj := subjBase + uint64(s) + key := x.DataKey(pred, subj) + pl, err := readTxn.Get(key) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantForwardObjs, got.Uids, + "forward list for subject %d (s=%d): missing some objects", subj, s) + } + + // Reverse: each object must hold exactly all nSubjects subjects. + wantReverseSubjs := make([]uint64, nSubjects) + for s := 0; s < nSubjects; s++ { + wantReverseSubjs[s] = subjBase + uint64(s) + } + for o := 0; o < nObjects; o++ { + obj := objBase + uint64(o) + key := x.ReverseKey(pred, obj) + pl, err := readTxn.Get(key) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantReverseSubjs, got.Uids, + "reverse list for object %d (o=%d): missing some subjects", obj, o) + } +} + +// TestPipelineReverseListCountMultiPred escalates further: multiple +// list-uid predicates with @reverse @count are mutated together in each +// batch, so the per-predicate pipeline goroutines for each predicate run +// in parallel inside Process(). This is the live-loader's actual shape +// (genre + director.film + starring + rated all live in the same payload). +func TestPipelineReverseListCountMultiPred(t *testing.T) { + dir, err := os.MkdirTemp("", "pipelinerevcount3_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + ps, err := badger.OpenManaged(badger.DefaultOptions(dir)) + require.NoError(t, err) + defer ps.Close() + posting.Init(ps, 0, false) + Init(ps) + posting.Oracle().ResetTxns() + + require.NoError(t, schema.ParseBytes([]byte(` + genre: [uid] @reverse @count . + starring: [uid] @reverse @count . + director_film: [uid] @reverse @count . + `), 1)) + + preds := []string{ + x.AttrInRootNamespace("genre"), + x.AttrInRootNamespace("starring"), + x.AttrInRootNamespace("director_film"), + } + + const ( + nSubjects = 30 + nObjects = 12 + ) + subjBase := uint64(10000) + objBase := uint64(20000) + + allEdges := make([]*pb.DirectedEdge, 0, len(preds)*nSubjects*nObjects) + for _, pred := range preds { + for s := 0; s < nSubjects; s++ { + for o := 0; o < nObjects; o++ { + allEdges = append(allEdges, &pb.DirectedEdge{ + Entity: subjBase + uint64(s), + Attr: pred, + ValueId: objBase + uint64(o), + ValueType: pb.Posting_UID, + Op: pb.DirectedEdge_SET, + }) + } + } + } + // Shuffle so each batch carries an interleaved mix of predicates. + rnd := rand.New(rand.NewSource(1)) + rnd.Shuffle(len(allEdges), func(i, j int) { + allEdges[i], allEdges[j] = allEdges[j], allEdges[i] + }) + + const batchSize = 23 + ts := uint64(10) + for start := 0; start < len(allEdges); start += batchSize { + end := start + batchSize + if end > len(allEdges) { + end = len(allEdges) + } + batch := allEdges[start:end] + + txn := posting.Oracle().RegisterStartTs(ts) + require.NoError(t, newRunMutations(context.Background(), batch, txn)) + txn.Update() + w := posting.NewTxnWriter(ps) + require.NoError(t, txn.CommitToDisk(w, ts+1)) + require.NoError(t, w.Flush()) + txn.UpdateCachedKeys(ts + 1) + ts += 2 + } + + readTxn := posting.Oracle().RegisterStartTs(math.MaxUint64) + + wantForwardObjs := make([]uint64, nObjects) + for o := 0; o < nObjects; o++ { + wantForwardObjs[o] = objBase + uint64(o) + } + wantReverseSubjs := make([]uint64, nSubjects) + for s := 0; s < nSubjects; s++ { + wantReverseSubjs[s] = subjBase + uint64(s) + } + + for _, pred := range preds { + for s := 0; s < nSubjects; s++ { + subj := subjBase + uint64(s) + pl, err := readTxn.Get(x.DataKey(pred, subj)) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantForwardObjs, got.Uids, + "forward list for pred=%q subj=%d", pred, subj) + } + for o := 0; o < nObjects; o++ { + obj := objBase + uint64(o) + pl, err := readTxn.Get(x.ReverseKey(pred, obj)) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantReverseSubjs, got.Uids, + "reverse list for pred=%q obj=%d", pred, obj) + } + } +} + +// TestPipelineReverseListCountConcurrent mirrors the live-loader's actual +// concurrency: many goroutines submitting batched transactions in parallel, +// going through a real conflict-checking commit (the fakeOracle harness +// also used by TestPipelineCountIndexConcurrent), against a [uid] @reverse +// @count predicate. This is the closest in-process reproduction of the +// systest/21million/live shape that we have without standing up a real +// cluster. +func TestPipelineReverseListCountConcurrent(t *testing.T) { + dir, err := os.MkdirTemp("", "pipelinerevcount4_") + require.NoError(t, err) + defer os.RemoveAll(dir) + + ps, err := badger.OpenManaged(badger.DefaultOptions(dir)) + require.NoError(t, err) + defer ps.Close() + posting.Init(ps, 0, false) + Init(ps) + posting.Oracle().ResetTxns() + + require.NoError(t, schema.ParseBytes( + []byte(`genre: [uid] @reverse @count .`), 1)) + + pred := x.AttrInRootNamespace("genre") + + const ( + nSubjects = 60 // films + nObjects = 25 // genres + ) + subjBase := uint64(10000) + objBase := uint64(20000) + + allEdges := make([]*pb.DirectedEdge, 0, nSubjects*nObjects) + for s := 0; s < nSubjects; s++ { + for o := 0; o < nObjects; o++ { + allEdges = append(allEdges, &pb.DirectedEdge{ + Entity: subjBase + uint64(s), + Attr: pred, + ValueId: objBase + uint64(o), + ValueType: pb.Posting_UID, + Op: pb.DirectedEdge_SET, + }) + } + } + rnd := rand.New(rand.NewSource(7)) + rnd.Shuffle(len(allEdges), func(i, j int) { + allEdges[i], allEdges[j] = allEdges[j], allEdges[i] + }) + + // Chunk into many small batches so multiple goroutines compete on the + // same predicate, mimicking `dgraph live -c 10` behavior. + const batchSize = 17 + type batch []*pb.DirectedEdge + batches := []batch{} + for start := 0; start < len(allEdges); start += batchSize { + end := start + batchSize + if end > len(allEdges) { + end = len(allEdges) + } + batches = append(batches, allEdges[start:end]) + } + + oracle := newFakeOracle(10) + const concurrency = 10 + + jobs := make(chan batch, len(batches)) + for _, b := range batches { + jobs <- b + } + close(jobs) + + var wg sync.WaitGroup + wg.Add(concurrency) + for w := 0; w < concurrency; w++ { + go func() { + defer wg.Done() + for b := range jobs { + // Retry until commit, with a fresh edge clone each attempt + // (the pipeline mutates edge.ValueId during processing). + for attempt := 0; attempt < 200; attempt++ { + clones := make([]*pb.DirectedEdge, len(b)) + for i, e := range b { + clones[i] = &pb.DirectedEdge{ + Entity: e.Entity, Attr: e.Attr, + ValueId: e.ValueId, ValueType: e.ValueType, Op: e.Op, + } + } + if runPipelineTxn(t, ps, oracle, clones) { + break + } + } + } + }() + } + wg.Wait() + + t.Logf("committed=%d aborted=%d", oracle.committed.Load(), oracle.aborted.Load()) + + readTxn := posting.Oracle().RegisterStartTs(math.MaxUint64) + + wantForwardObjs := make([]uint64, nObjects) + for o := 0; o < nObjects; o++ { + wantForwardObjs[o] = objBase + uint64(o) + } + wantReverseSubjs := make([]uint64, nSubjects) + for s := 0; s < nSubjects; s++ { + wantReverseSubjs[s] = subjBase + uint64(s) + } + + missingForward := []string{} + missingReverse := []string{} + for s := 0; s < nSubjects; s++ { + subj := subjBase + uint64(s) + pl, err := readTxn.Get(x.DataKey(pred, subj)) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + if len(got.Uids) != nObjects { + missingForward = append(missingForward, + fmt.Sprintf("subj=%d has %d/%d", subj, len(got.Uids), nObjects)) + } + } + for o := 0; o < nObjects; o++ { + obj := objBase + uint64(o) + pl, err := readTxn.Get(x.ReverseKey(pred, obj)) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + if len(got.Uids) != nSubjects { + missingReverse = append(missingReverse, + fmt.Sprintf("obj=%d has %d/%d", obj, len(got.Uids), nSubjects)) + } + } + require.Empty(t, missingForward, "forward lists missing entries") + require.Empty(t, missingReverse, "reverse lists missing entries") + + // And the strict version: each list must match exactly. + for s := 0; s < nSubjects; s++ { + subj := subjBase + uint64(s) + pl, err := readTxn.Get(x.DataKey(pred, subj)) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantForwardObjs, got.Uids, + "forward list for subj %d", subj) + } + for o := 0; o < nObjects; o++ { + obj := objBase + uint64(o) + pl, err := readTxn.Get(x.ReverseKey(pred, obj)) + require.NoError(t, err) + got, err := pl.Uids(posting.ListOptions{ReadTs: math.MaxUint64}) + require.NoError(t, err) + require.ElementsMatch(t, wantReverseSubjs, got.Uids, + "reverse list for obj %d", obj) + } +} + func TestDeleteSetWithVarEdgeCorruptsData(t *testing.T) { // Setup temporary directory for Badger DB dir, err := os.MkdirTemp("", "storetest_") @@ -119,10 +1073,10 @@ func TestDeleteSetWithVarEdgeCorruptsData(t *testing.T) { uidRoom := uint64(1) uidJohn := uint64(2) - runMutation := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { + newRunMutation := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { txn := posting.Oracle().RegisterStartTs(startTs) for _, edge := range edges { - require.NoError(t, runMutation(ctx, edge, txn)) + require.NoError(t, newRunMutation(ctx, edge, txn)) } txn.Update() writer := posting.NewTxnWriter(ps) @@ -132,7 +1086,7 @@ func TestDeleteSetWithVarEdgeCorruptsData(t *testing.T) { } // Initial mutation: Set John → Leopard - runMutation(1, 3, []*pb.DirectedEdge{ + newRunMutation(1, 3, []*pb.DirectedEdge{ { Entity: uidJohn, Attr: attrPerson, @@ -162,7 +1116,7 @@ func TestDeleteSetWithVarEdgeCorruptsData(t *testing.T) { // Second mutation: Remove John from Leopard, assign Amanda uidAmanda := uint64(3) - runMutation(6, 8, []*pb.DirectedEdge{ + newRunMutation(6, 8, []*pb.DirectedEdge{ { Entity: uidJohn, Attr: attrOffice, @@ -225,7 +1179,7 @@ func TestGetScalarList(t *testing.T) { runM := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { txn := posting.Oracle().RegisterStartTs(startTs) for _, edge := range edges { - x.Check(runMutation(context.Background(), edge, txn)) + x.Check(newRunMutation(context.Background(), edge, txn)) } txn.Update() writer := posting.NewTxnWriter(pstore) @@ -283,7 +1237,7 @@ func TestMultipleTxnListCount(t *testing.T) { runM := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { txn := posting.Oracle().RegisterStartTs(startTs) for _, edge := range edges { - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) } txn.Update() writer := posting.NewTxnWriter(pstore) @@ -342,7 +1296,7 @@ func TestScalarPredicateRevCount(t *testing.T) { runM := func(startTs, commitTs uint64, edges []*pb.DirectedEdge) { txn := posting.Oracle().RegisterStartTs(startTs) for _, edge := range edges { - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) } txn.Update() writer := posting.NewTxnWriter(pstore) @@ -423,7 +1377,7 @@ func TestScalarPredicateIntCount(t *testing.T) { runM := func(startTs, commitTs uint64, edge *pb.DirectedEdge) { txn := posting.Oracle().RegisterStartTs(startTs) - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) txn.Update() writer := posting.NewTxnWriter(pstore) require.NoError(t, txn.CommitToDisk(writer, commitTs)) @@ -477,7 +1431,7 @@ func TestScalarPredicateCount(t *testing.T) { runM := func(startTs, commitTs uint64, edge *pb.DirectedEdge) { txn := posting.Oracle().RegisterStartTs(startTs) - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) txn.Update() writer := posting.NewTxnWriter(pstore) require.NoError(t, txn.CommitToDisk(writer, commitTs)) @@ -531,7 +1485,7 @@ func TestSingleUidReplacement(t *testing.T) { attr := x.AttrInRootNamespace("singleUidReplaceTest") // Txn 1. Set 1 -> 2 - x.Check(runMutation(ctx, &pb.DirectedEdge{ + x.Check(newRunMutation(ctx, &pb.DirectedEdge{ ValueId: 2, Attr: attr, Entity: 1, @@ -547,7 +1501,7 @@ func TestSingleUidReplacement(t *testing.T) { // Txn 2. Set 1 -> 3 txn = posting.Oracle().RegisterStartTs(9) - x.Check(runMutation(ctx, &pb.DirectedEdge{ + x.Check(newRunMutation(ctx, &pb.DirectedEdge{ ValueId: 3, Attr: attr, Entity: 1, @@ -591,14 +1545,14 @@ func TestSingleString(t *testing.T) { attr := x.AttrInRootNamespace("singleUidTest") // Txn 1. Set 1 -> david 2 -> blush - x.Check(runMutation(ctx, &pb.DirectedEdge{ + x.Check(newRunMutation(ctx, &pb.DirectedEdge{ Value: []byte("david"), Attr: attr, Entity: 1, Op: pb.DirectedEdge_SET, }, txn)) - x.Check(runMutation(ctx, &pb.DirectedEdge{ + x.Check(newRunMutation(ctx, &pb.DirectedEdge{ Value: []byte("blush"), Attr: attr, Entity: 2, @@ -614,14 +1568,14 @@ func TestSingleString(t *testing.T) { // Txn 2. Set 2 -> david 1 -> blush txn = posting.Oracle().RegisterStartTs(9) - x.Check(runMutation(ctx, &pb.DirectedEdge{ + x.Check(newRunMutation(ctx, &pb.DirectedEdge{ Value: []byte("david"), Attr: attr, Entity: 2, Op: pb.DirectedEdge_SET, }, txn)) - x.Check(runMutation(ctx, &pb.DirectedEdge{ + x.Check(newRunMutation(ctx, &pb.DirectedEdge{ Value: []byte("blush"), Attr: attr, Entity: 1, @@ -693,7 +1647,7 @@ func TestLangExact(t *testing.T) { Lang: "en", } - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) edge = &pb.DirectedEdge{ Value: []byte("hindi"), @@ -703,7 +1657,7 @@ func TestLangExact(t *testing.T) { Lang: "hi", } - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) txn.Update() writer := posting.NewTxnWriter(pstore) @@ -745,7 +1699,6 @@ func BenchmarkAddMutationWithIndex(b *testing.B) { posting.Init(ps, 0, false) Init(ps) err = schema.ParseBytes([]byte("benchmarkadd: string @index(term) ."), 1) - fmt.Println(err) if err != nil { panic(err) } @@ -767,7 +1720,7 @@ func BenchmarkAddMutationWithIndex(b *testing.B) { Op: pb.DirectedEdge_SET, } - x.Check(runMutation(ctx, edge, txn)) + x.Check(newRunMutation(ctx, edge, txn)) } } diff --git a/x/config.go b/x/config.go index 15f4123c17e..8201bb40702 100644 --- a/x/config.go +++ b/x/config.go @@ -142,6 +142,19 @@ type WorkerOptions struct { HardSync bool // Audit contains the audit flags that enables the audit. Audit bool + // MutationsPipelineThreshold gates the per-predicate mutation pipeline + // in applyMutations. A mutation runs through the pipeline only when + // MutationsPipelineThreshold > 0 and len(m.Edges) >= the threshold; + // otherwise it falls back to the legacy serial path. Set to 0 to + // disable the pipeline entirely. Set to 1 (default) to always use it. + // The pipeline pays goroutine spin-up cost per predicate, so tiny + // mutations are slower on it; bulk multi-predicate mutations are + // faster — set to a value above the per-mutation edge count where the + // crossover happens for your workload (~100 in benchmarks here) if + // you want only large mutations to take the pipeline path. + // Plumbed via the "feature-flags" superflag as + // "mutations-pipeline-threshold". + MutationsPipelineThreshold int } // WorkerConfig stores the global instance of the worker package's options.