release/v20.11: fix(dgraph): Fixing multiple race conditions (#7278)

* fix(race): fixing race condition in localcache (#7140) * fixing race condition in localcache * name refactor * fixing race in pick postings + fixing race log in t.go (#7149) * fixing race condition in process subgraph (#7156) * fixing race condition in disk storage sync and save (#7263) * fixing bgindex + fix alpha z.closer (#7266)
hypermodeinc · Jan 12, 2021 · 677706b · 677706b
1 parent 8c44001
commit 677706b
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 155 deletions.
diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go
@@ -526,7 +526,7 @@ func setupServer(closer *z.Closer) {
 	http.HandleFunc("/ui/keywords", keywordHandler)
 
 	// Initialize the servers.
-	admin.ServerCloser = z.NewCloser(3)
+	admin.ServerCloser.AddRunning(3)
 	go serveGRPC(grpcListener, tlsCfg, admin.ServerCloser)
 	go x.StartListenHttpAndHttps(httpListener, tlsCfg, admin.ServerCloser)
 
@@ -730,11 +730,6 @@ func run() {
 		var numShutDownSig int
 		for range sdCh {
 			closer := admin.ServerCloser
-			if closer == nil {
-				glog.Infoln("Caught Ctrl-C. Terminating now.")
-				os.Exit(1)
-			}
-
 			select {
 			case <-closer.HasBeenClosed():
 			default:

diff --git a/dgraph/cmd/zero/assign.go b/dgraph/cmd/zero/assign.go
@@ -196,7 +196,7 @@ func (s *Server) AssignUids(ctx context.Context, num *pb.Num) (*pb.AssignedIds,
 
 	select {
 	case <-ctx.Done():
-		return reply, ctx.Err()
+		return &emptyAssignedIds, ctx.Err()
 	case err := <-c:
 		span.Annotatef(nil, "Error while leasing %+v: %v", num, err)
 		return reply, err

diff --git a/graphql/admin/shutdown.go b/graphql/admin/shutdown.go
@@ -28,7 +28,7 @@ import (
 var (
 	// ServerCloser is used to signal and wait for other goroutines to return gracefully after user
 	// requests shutdown.
-	ServerCloser *z.Closer
+	ServerCloser = z.NewCloser(0)
 )
 
 func resolveShutdown(ctx context.Context, m schema.Mutation) (*resolve.Resolved, bool) {

diff --git a/posting/list.go b/posting/list.go
@@ -544,12 +544,9 @@ func (l *List) addMutationInternal(ctx context.Context, txn *Txn, t *pb.Directed
 
 // getMutation returns a marshaled version of posting list mutation stored internally.
 func (l *List) getMutation(startTs uint64) []byte {
-	l.Lock()
-	defer l.Unlock()
+	l.RLock()
+	defer l.RUnlock()
 	if pl, ok := l.mutationMap[startTs]; ok {
-		for _, p := range pl.GetPostings() {
-			p.StartTs = 0
-		}
 		data, err := pl.Marshal()
 		x.Check(err)
 		return data
@@ -617,7 +614,6 @@ func (l *List) pickPostings(readTs uint64) (uint64, []*pb.Posting) {
 					deleteBelowTs = effectiveTs
 					continue
 				}
-				mpost.StartTs = startTs
 				posts = append(posts, mpost)
 			}
 		}

diff --git a/posting/lists.go b/posting/lists.go
@@ -154,9 +154,19 @@ func (lc *LocalCache) SetIfAbsent(key string, updated *List) *List {
 }
 
 func (lc *LocalCache) getInternal(key []byte, readFromDisk bool) (*List, error) {
-	if lc.plists == nil {
-		return getNew(key, pstore, lc.startTs)
+	getNewPlistNil := func() (*List, error){
+		lc.RLock()
+		defer lc.RUnlock()
+		if lc.plists == nil {
+			return getNew(key, pstore, lc.startTs)
+		}
+		return nil, nil
+	}
+
+	if l, err := getNewPlistNil(); l != nil || err != nil {
+		return l, err
 	}
+
 	skey := string(key)
 	if pl := lc.getNoStore(skey); pl != nil {
 		return pl, nil

diff --git a/query/query.go b/query/query.go
@@ -1961,7 +1961,11 @@ func expandSubgraph(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
 			temp.Params.IsInternal = false
 			temp.Params.Expand = ""
 			temp.Params.Facet = &pb.FacetParams{AllKeys: true}
-			temp.Filters = child.Filters
+			for _, cf := range child.Filters {
+				s := &SubGraph{}
+				recursiveCopy(s, cf)
+				temp.Filters = append(temp.Filters, s)
+			}
 
 			// Go through each child, create a copy and attach to temp.Children.
 			for _, cc := range child.Children {

diff --git a/raftwal/storage.go b/raftwal/storage.go
@@ -374,6 +374,9 @@ func (w *DiskStorage) NumLogFiles() int {
 
 // Sync calls the Sync method in the underlying badger instance to write all the contents to disk.
 func (w *DiskStorage) Sync() error {
+	w.lock.Lock()
+	defer w.lock.Unlock()
+
 	if err := w.meta.Sync(); err != nil {
 		return errors.Wrapf(err, "while syncing meta")
 	}

diff --git a/systest/bgindex/docker-compose.yml b/systest/bgindex/docker-compose.yml
@@ -19,109 +19,8 @@ services:
       source: ../../ee/acl/hmac-secret
       target: /secret/hmac
       read_only: true
-    command: /gobin/dgraph alpha --my=alpha1:7080 --zero=zero1:5080,zero2:5080,zero3:5080
+    command: /gobin/dgraph alpha --my=alpha1:7080 --zero=zero1:5080
       --logtostderr -v=2 --whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --acl_secret_file=/secret/hmac
-      --acl_access_ttl=300s
-  alpha2:
-    image: dgraph/dgraph:latest
-    working_dir: /data/alpha2
-    labels:
-      cluster: test
-    ports:
-    - "8080"
-    - "9080"
-    volumes:
-    - type: bind
-      source: $GOPATH/bin
-      target: /gobin
-      read_only: true
-    - type: bind
-      source: ../../ee/acl/hmac-secret
-      target: /secret/hmac
-      read_only: true
-    command: /gobin/dgraph alpha --my=alpha2:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --acl_secret_file=/secret/hmac
-      --acl_access_ttl=300s
-  alpha3:
-    image: dgraph/dgraph:latest
-    working_dir: /data/alpha3
-    labels:
-      cluster: test
-    ports:
-    - "8080"
-    - "9080"
-    volumes:
-    - type: bind
-      source: $GOPATH/bin
-      target: /gobin
-      read_only: true
-    - type: bind
-      source: ../../ee/acl/hmac-secret
-      target: /secret/hmac
-      read_only: true
-    command: /gobin/dgraph alpha --my=alpha3:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --acl_secret_file=/secret/hmac
-      --acl_access_ttl=300s
-  alpha4:
-    image: dgraph/dgraph:latest
-    working_dir: /data/alpha4
-    labels:
-      cluster: test
-    ports:
-    - "8080"
-    - "9080"
-    volumes:
-    - type: bind
-      source: $GOPATH/bin
-      target: /gobin
-      read_only: true
-    - type: bind
-      source: ../../ee/acl/hmac-secret
-      target: /secret/hmac
-      read_only: true
-    command: /gobin/dgraph alpha --my=alpha4:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --acl_secret_file=/secret/hmac
-      --acl_access_ttl=300s
-  alpha5:
-    image: dgraph/dgraph:latest
-    working_dir: /data/alpha5
-    labels:
-      cluster: test
-    ports:
-    - "8080"
-    - "9080"
-    volumes:
-    - type: bind
-      source: $GOPATH/bin
-      target: /gobin
-      read_only: true
-    - type: bind
-      source: ../../ee/acl/hmac-secret
-      target: /secret/hmac
-      read_only: true
-    command: /gobin/dgraph alpha --my=alpha5:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --acl_secret_file=/secret/hmac
-      --acl_access_ttl=300s
-  alpha6:
-    image: dgraph/dgraph:latest
-    working_dir: /data/alpha6
-    labels:
-      cluster: test
-    ports:
-    - "8080"
-    - "9080"
-    volumes:
-    - type: bind
-      source: $GOPATH/bin
-      target: /gobin
-      read_only: true
-    - type: bind
-      source: ../../ee/acl/hmac-secret
-      target: /secret/hmac
-      read_only: true
-    command: /gobin/dgraph alpha --my=alpha6:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --acl_secret_file=/secret/hmac
-      --acl_access_ttl=300s
   zero1:
     image: dgraph/dgraph:latest
     working_dir: /data/zero1
@@ -135,40 +34,5 @@ services:
       source: $GOPATH/bin
       target: /gobin
       read_only: true
-    command: /gobin/dgraph zero --idx=1 --my=zero1:5080 --replicas=3 --logtostderr
-      -v=2 --bindall
-  zero2:
-    image: dgraph/dgraph:latest
-    working_dir: /data/zero2
-    depends_on:
-    - zero1
-    labels:
-      cluster: test
-    ports:
-    - "5080"
-    - "6080"
-    volumes:
-    - type: bind
-      source: $GOPATH/bin
-      target: /gobin
-      read_only: true
-    command: /gobin/dgraph zero --idx=2 --my=zero2:5080 --replicas=3 --logtostderr
-      -v=2 --peer=zero1:5080
-  zero3:
-    image: dgraph/dgraph:latest
-    working_dir: /data/zero3
-    depends_on:
-    - zero2
-    labels:
-      cluster: test
-    ports:
-    - "5080"
-    - "6080"
-    volumes:
-    - type: bind
-      source: $GOPATH/bin
-      target: /gobin
-      read_only: true
-    command: /gobin/dgraph zero --idx=3 --my=zero3:5080 --replicas=3 --logtostderr
-      -v=2 --peer=zero1:5080
+    command: /gobin/dgraph zero --idx=1 --my=zero1:5080 --logtostderr -v=2 --bindall
 volumes: {}