Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions dgraph/cmd/alpha/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion dgraph/cmd/zero/assign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion graphql/admin/shutdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 2 additions & 6 deletions posting/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -617,7 +614,6 @@ func (l *List) pickPostings(readTs uint64) (uint64, []*pb.Posting) {
deleteBelowTs = effectiveTs
continue
}
mpost.StartTs = startTs
posts = append(posts, mpost)
}
}
Expand Down
14 changes: 12 additions & 2 deletions posting/lists.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions raftwal/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
140 changes: 2 additions & 138 deletions systest/bgindex/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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: {}