Conversation
jarifibrahim
left a comment
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @manishrjain, @parasssh, and @vvbalaji-dgraph)
x/sentry_integration.go, line 108 at r1 (raw file):
} // WriteCidFile writes the CID to a well-known location so it can be read and sent to Sentry on panic.
1000 char
x/sentry_integration.go, line 115 at r1 (raw file):
err := ioutil.WriteFile(filePath, []byte(cid), 0644) if err != nil {
if err := ...
x/sentry_integration.go, line 121 at r1 (raw file):
} // readAndRemoveCidFile reads the file from a well-known location so it can be read and sent to Sentry on panic.
100 characters
x/sentry_integration.go, line 151 at r1 (raw file):
func PanicHandler(out string) { cid := readAndRemoveCidFile() if cid != "" {
if cid := ...
martinmr
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @manishrjain, @parasssh, and @vvbalaji-dgraph)
dgraph/cmd/zero/raft.go, line 511 at r1 (raw file):
if err == nil { glog.Infof("CID set for cluster: %v", id) x.WriteCidFile(id)
is it always true that the cluster id will be id? What if another zero already proposed a different cluster ID and this proposal is ignored?
I think it'd be better here to read the cid directly from the cluster state, just to be safe.
x/sentry_integration.go, line 34 at r1 (raw file):
filePath
rename this to cidPath or something more specific.
x/sentry_integration.go, line 116 at r1 (raw file):
Infof
this should be a warning. Same for the other ones.
jarifibrahim
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @manishrjain, @parasssh, and @vvbalaji-dgraph)
x/sentry_integration.go, line 105 at r1 (raw file):
}) filePath = os.TempDir() + "/" + "dgraph-" + subcmd + "-cid-sentry" // e.g. /tmp/dgraph-alpha-cid-sentry
100 char
x/sentry_integration.go, line 108 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
1000 char
100 char
parasssh
left a comment
There was a problem hiding this comment.
Dismissed @martinmr from a discussion.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @jarifibrahim, @manishrjain, @martinmr, and @vvbalaji-dgraph)
dgraph/cmd/zero/raft.go, line 511 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
is it always true that the cluster id will be
id? What if another zero already proposed a different cluster ID and this proposal is ignored?I think it'd be better here to read the cid directly from the cluster state, just to be safe.
Not needed. The proposeAndWait( ) will only allow proposals from the leader to go through.
x/sentry_integration.go, line 34 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
filePathrename this to cidPath or something more specific.
Done.
x/sentry_integration.go, line 105 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
100 char
Done.
x/sentry_integration.go, line 108 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
100 char
Done.
x/sentry_integration.go, line 115 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
if err := ...
Done.
x/sentry_integration.go, line 116 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
Infofthis should be a warning. Same for the other ones.
Done.
x/sentry_integration.go, line 121 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
100 characters
Done.
x/sentry_integration.go, line 151 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
if cid := ...
Done.
parasssh
left a comment
There was a problem hiding this comment.
Dismissed @martinmr from 2 discussions.
Reviewable status: 4 of 5 files reviewed, 5 unresolved discussions (waiting on @jarifibrahim, @manishrjain, @martinmr, and @vvbalaji-dgraph)
(cherry picked from commit 9765e3f)
Using the file system to share the CID info between processes.
This change is
Docs Preview: