[Breaking] fix(Flags): consolidation and help text improvements (#7544)…

… (#7560) * small SuperFlag --help fixes * update Ristretto * combine telemetry flags * combine telemetry flags * cleanup enable_sentry * minor fixes * get tls working for now * move --mutations flag to --limit superflag * fix limit default * move max_retries to badger superflag * critical path helper * fix * --help fixes * always show default string in --help * clean up cdc flag handling * move cdc defaults outside of _ee * telemetry documentation * cleaning * fix comment * clean up tls defaults * nevermind * removing some extra fields Co-authored-by: aman-bansal <[email protected]> Co-authored-by: Karl McGuire <[email protected]>
hypermodeinc · Mar 12, 2021 · a7cac5d · a7cac5d
1 parent f8a4d53
commit a7cac5d
Show file tree

Hide file tree

Showing 15 changed files with 104 additions and 101 deletions.
diff --git a/dgraph/cmd/alpha/mutations_mode/docker-compose.yml b/dgraph/cmd/alpha/mutations_mode/docker-compose.yml
@@ -1,5 +1,5 @@
 # Auto-generated with: [/home/mrjn/go/bin/compose -a3 -z3 --mem= --names=false -o=0 --expose_ports=false]
-# And manually modified to add --mutations flags in Alphas.
+# And manually modified to add --limit "mutations=<mode>;" flags in Alphas.
 #
 version: "3.5"
 services:
@@ -17,8 +17,9 @@ services:
       target: /gobin
       read_only: true
     command: /gobin/dgraph alpha --my=alpha1:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --mutations=disallow
+      --logtostderr -v=2
       --security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
+      --limit "mutations=disallow;"
   alpha2:
     image: dgraph/dgraph:latest
     working_dir: /data/alpha2
@@ -33,8 +34,9 @@ services:
       target: /gobin
       read_only: true
     command: /gobin/dgraph alpha --my=alpha2:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --mutations=strict 
+      --logtostderr -v=2
       --security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
+      --limit "mutations=strict;"
   alpha3:
     image: dgraph/dgraph:latest
     working_dir: /data/alpha3
@@ -49,8 +51,9 @@ services:
       target: /gobin
       read_only: true
     command: /gobin/dgraph alpha --my=alpha3:7080 --zero=zero1:5080,zero2:5080,zero3:5080
-      --logtostderr -v=2 --mutations=strict
+      --logtostderr -v=2
       --security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
+      --limit "mutations=strict;"
   zero1:
     image: dgraph/dgraph:latest
     working_dir: /data/zero1
@@ -64,7 +67,7 @@ services:
       source: $GOPATH/bin
       target: /gobin
       read_only: true
-    command: /gobin/dgraph zero --raft="idx=1" --my=zero1:5080 --replicas=1 --logtostderr
+    command: /gobin/dgraph zero --raft "idx=1;" --my=zero1:5080 --replicas=1 --logtostderr
       -v=2 --bindall
   zero2:
     image: dgraph/dgraph:latest
@@ -81,7 +84,7 @@ services:
       source: $GOPATH/bin
       target: /gobin
       read_only: true
-    command: /gobin/dgraph zero --raft="idx=2" --my=zero2:5080 --replicas=1 --logtostderr
+    command: /gobin/dgraph zero --raft "idx=2;" --my=zero2:5080 --replicas=1 --logtostderr
       -v=2 --peer=zero1:5080
   zero3:
     image: dgraph/dgraph:latest
@@ -98,6 +101,6 @@ services:
       source: $GOPATH/bin
       target: /gobin
       read_only: true
-    command: /gobin/dgraph zero --raft="idx=3" --my=zero3:5080 --replicas=1 --logtostderr
+    command: /gobin/dgraph zero --raft "idx=3;" --my=zero3:5080 --replicas=1 --logtostderr
       -v=2 --peer=zero1:5080
 volumes: {}
diff --git a/dgraph/cmd/alpha/mutations_mode/mutations_mode_test.go b/dgraph/cmd/alpha/mutations_mode/mutations_mode_test.go
@@ -29,7 +29,7 @@ import (
 	"google.golang.org/grpc"
 )
 
-// Tests in this file require a cluster running with the --mutations=<mode> option.
+// Tests in this file require a cluster running with the --limit "mutations=<mode>;" flag.
 
 func runOn(conn *grpc.ClientConn, fn func(*testing.T, *dgo.Dgraph)) func(*testing.T) {
 	return func(t *testing.T) {

diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go
@@ -125,20 +125,13 @@ they form a Raft group and provide synchronous replication.
 	flag.StringP("zero", "z", fmt.Sprintf("localhost:%d", x.PortZeroGrpc),
 		"Comma separated list of Dgraph Zero addresses of the form IP_ADDRESS:PORT.")
 
-	flag.Int("max_retries", -1,
-		"Commits to disk will give up after these number of retries to prevent locking the worker"+
-			" in a failed state. Use -1 to retry infinitely.")
-
 	// Useful for running multiple servers on the same machine.
 	flag.IntP("port_offset", "o", 0,
 		"Value added to all listening port numbers. [Internal=7080, HTTP=8080, Grpc=9080]")
 
-	//Custom plugins.
+	// Custom plugins.
 	flag.String("custom_tokenizers", "",
-		"Comma separated list of tokenizer plugins")
-
-	flag.String("mutations", "allow",
-		"Set mutation mode to allow, disallow, or strict.")
+		"Comma separated list of tokenizer plugins for custom indices.")
 
 	// By default Go GRPC traces all requests.
 	grpc.EnableTracing = false
@@ -151,6 +144,9 @@ they form a Raft group and provide synchronous replication.
 			compression, while "zstd:1" would set zstd compression at level 1.`).
 		Flag("goroutines",
 			"The number of goroutines to use in badger.Stream.").
+		Flag("max-retries",
+			"Commits to disk will give up after these number of retries to prevent locking the "+
+				"worker in a failed state. Use -1 to retry infinitely.").
 		String())
 
 	flag.String("raft", worker.RaftDefaults, z.NewSuperFlagHelp(worker.RaftDefaults).
@@ -201,14 +197,16 @@ they form a Raft group and provide synchronous replication.
 		Flag("normalize-node",
 			"The maximum number of nodes that can be returned in a query that uses the normalize "+
 				"directive.").
+		Flag("mutations",
+			"[allow, disallow, strict] The mutations mode to use.").
 		Flag("mutations-nquad",
 			"The maximum number of nquads that can be inserted in a mutation request.").
 		String())
 
 	flag.String("ludicrous", worker.LudicrousDefaults, z.NewSuperFlagHelp(worker.LudicrousDefaults).
 		Head("Ludicrous options").
 		Flag("enabled",
-			"Run Dgraph in Ludicrous mode.").
+			"Set enabled to true to run Dgraph in Ludicrous mode.").
 		Flag("concurrency",
 			"The number of concurrent threads to use in Ludicrous mode.").
 		String())
@@ -228,7 +226,7 @@ they form a Raft group and provide synchronous replication.
 			"The URL of a lambda server that implements custom GraphQL Javascript resolvers.").
 		String())
 
-	flag.String("cdc", "", z.NewSuperFlagHelp(worker.CDCDefaults).
+	flag.String("cdc", worker.CDCDefaults, z.NewSuperFlagHelp(worker.CDCDefaults).
 		Head("Change Data Capture options").
 		Flag("file",
 			"The path where audit logs will be stored.").
@@ -246,7 +244,7 @@ they form a Raft group and provide synchronous replication.
 			"The path to client key file for TLS encryption.").
 		String())
 
-	flag.String("audit", "", z.NewSuperFlagHelp(worker.AuditDefaults).
+	flag.String("audit", worker.AuditDefaults, z.NewSuperFlagHelp(worker.AuditDefaults).
 		Head("Audit options").
 		Flag("output",
 			`[stdout, /path/to/dir] This specifies where audit logs should be output to.
@@ -564,10 +562,6 @@ func setupServer(closer *z.Closer) {
 	go serveGRPC(grpcListener, tlsCfg, x.ServerCloser)
 	go x.StartListenHttpAndHttps(httpListener, tlsCfg, x.ServerCloser)
 
-	if Alpha.Conf.GetBool("telemetry") {
-		go edgraph.PeriodicallyPostTelemetry()
-	}
-
 	go func() {
 		defer x.ServerCloser.Done()
 
@@ -594,13 +588,17 @@ func setupServer(closer *z.Closer) {
 
 func run() {
 	var err error
-	if Alpha.Conf.GetBool("enable_sentry") {
+
+	telemetry := z.NewSuperFlag(Alpha.Conf.GetString("telemetry")).MergeAndCheckDefault(
+		x.TelemetryDefaults)
+	if telemetry.GetBool("sentry") {
 		x.InitSentry(enc.EeBuild)
 		defer x.FlushSentry()
 		x.ConfigureSentryScope("alpha")
 		x.WrapPanics()
 		x.SentryOptOutNote()
 	}
+
 	bindall = Alpha.Conf.GetBool("bindall")
 
 	totalCache := int64(Alpha.Conf.GetInt("cache_mb"))
@@ -654,15 +652,17 @@ func run() {
 		glog.Info("ACL secret key loaded successfully.")
 	}
 
-	switch strings.ToLower(Alpha.Conf.GetString("mutations")) {
+	x.Config.Limit = z.NewSuperFlag(Alpha.Conf.GetString("limit")).MergeAndCheckDefault(
+		worker.LimitDefaults)
+	switch strings.ToLower(x.Config.Limit.GetString("mutations")) {
 	case "allow":
 		opts.MutationsMode = worker.AllowMutations
 	case "disallow":
 		opts.MutationsMode = worker.DisallowMutations
 	case "strict":
 		opts.MutationsMode = worker.StrictMutations
 	default:
-		glog.Error("--mutations argument must be one of allow, disallow, or strict")
+		glog.Error(`--limit "mutations=<mode>;" must be one of allow, disallow, or strict`)
 		os.Exit(1)
 	}
 
@@ -688,7 +688,6 @@ func run() {
 		ZeroAddr:            strings.Split(Alpha.Conf.GetString("zero"), ","),
 		Raft:                raft,
 		WhiteListedIPRanges: ips,
-		MaxRetries:          Alpha.Conf.GetInt("max_retries"),
 		StrictMutations:     opts.MutationsMode == worker.StrictMutations,
 		AclEnabled:          aclKey != nil,
 		AbortOlderThan:      abortDur,
@@ -704,6 +703,10 @@ func run() {
 	}
 	x.WorkerConfig.Parse(Alpha.Conf)
 
+	if telemetry.GetBool("reports") {
+		go edgraph.PeriodicallyPostTelemetry()
+	}
+
 	// Set the directory for temporary buffers.
 	z.SetTmpDir(x.WorkerConfig.TmpDir)
 
@@ -712,8 +715,6 @@ func run() {
 	setupCustomTokenizers()
 	x.Init()
 	x.Config.PortOffset = Alpha.Conf.GetInt("port_offset")
-	x.Config.Limit = z.NewSuperFlag(Alpha.Conf.GetString("limit")).MergeAndCheckDefault(
-		worker.LimitDefaults)
 	x.Config.LimitMutationsNquad = int(x.Config.Limit.GetInt64("mutations-nquad"))
 	x.Config.LimitQueryEdge = x.Config.Limit.GetUint64("query-edge")
 

diff --git a/dgraph/cmd/bulk/run.go b/dgraph/cmd/bulk/run.go
@@ -47,7 +47,8 @@ var Bulk x.SubCommand
 
 var defaultOutDir = "./out"
 
-const bulkBadgerDefaults = " cache_mb=64; cache_percentage=70,30;"
+const BulkBadgerDefaults = worker.BadgerDefaults +
+	" cache_mb=64; cache_percentage=70,30;"
 
 func init() {
 	Bulk.Cmd = &cobra.Command{
@@ -120,32 +121,28 @@ func init() {
 	flag.Uint64("force-namespace", math.MaxUint64,
 		"Namespace onto which to load the data. If not set, will preserve the namespace.")
 
-	// Bulk has some extra defaults for Badger SuperFlag. These should only be applied in this
-	// package.
-	flag.String("badger", worker.BadgerDefaults+bulkBadgerDefaults,
-		z.NewSuperFlagHelp(worker.BadgerDefaults+bulkBadgerDefaults).
-			Head("Badger options").
-			Flag("compression",
-				"Specifies the compression algorithm and compression level (if applicable) for the "+
-					`postings directory. "none" would disable compression, while "zstd:1" would set `+
-					"zstd compression at level 1.").
-			Flag("goroutines",
-				"The number of goroutines to use in badger.Stream.").
-			Flag("cache-mb",
-				"Total size of cache (in MB) per shard in the reducer.").
-			Flag("cache-percentage",
-				"Cache percentages summing up to 100 for various caches. (FORMAT: BlockCacheSize,"+
-					"IndexCacheSize)").
-			String())
+	flag.String("badger", BulkBadgerDefaults, z.NewSuperFlagHelp(BulkBadgerDefaults).
+		Head("Badger options").
+		Flag("compression",
+			"Specifies the compression algorithm and compression level (if applicable) for the "+
+				`postings directory. "none" would disable compression, while "zstd:1" would set `+
+				"zstd compression at level 1.").
+		Flag("goroutines",
+			"The number of goroutines to use in badger.Stream.").
+		Flag("cache-mb",
+			"Total size of cache (in MB) per shard in the reducer.").
+		Flag("cache-percentage",
+			"Cache percentages summing up to 100 for various caches. (FORMAT: BlockCacheSize,"+
+				"IndexCacheSize)").
+		String())
 
 	x.RegisterClientTLSFlags(flag)
 	// Encryption and Vault options
 	enc.RegisterFlags(flag)
 }
 
 func run() {
-	badger := z.NewSuperFlag(Bulk.Conf.GetString("badger")).MergeAndCheckDefault(
-		worker.BadgerDefaults + bulkBadgerDefaults)
+	badger := z.NewSuperFlag(Bulk.Conf.GetString("badger")).MergeAndCheckDefault(BulkBadgerDefaults)
 	ctype, clevel := x.ParseCompression(badger.GetString("compression"))
 	opt := options{
 		DataFiles:        Bulk.Conf.GetString("files"),

diff --git a/dgraph/cmd/zero/run.go b/dgraph/cmd/zero/run.go
@@ -30,6 +30,7 @@ import (
 	"time"
 
 	"github.com/dgraph-io/dgraph/ee/audit"
+	"github.com/dgraph-io/dgraph/worker"
 
 	"go.opencensus.io/plugin/ocgrpc"
 	otrace "go.opencensus.io/trace"
@@ -49,9 +50,10 @@ import (
 )
 
 type options struct {
+	raft              *z.SuperFlag
+	telemetry         *z.SuperFlag
 	bindall           bool
 	portOffset        int
-	Raft              *z.SuperFlag
 	numReplicas       int
 	peer              string
 	w                 string
@@ -106,9 +108,7 @@ instances to achieve high-availability.
 				"in Raft elections. This can be used to achieve a read-only replica.").
 		String())
 
-	// NOTE: audit needs an empty default string otherwise it would panic with an empty "dir"
-	//       option.
-	flag.String("audit", "", z.NewSuperFlagHelp("").
+	flag.String("audit", worker.AuditDefaults, z.NewSuperFlagHelp(worker.AuditDefaults).
 		Head("Audit options").
 		Flag("output",
 			`[stdout, /path/to/dir] This specifies where audit logs should be output to.
@@ -154,12 +154,12 @@ func (st *state) serveGRPC(l net.Listener, store *raftwal.DiskStorage) {
 	}
 	s := grpc.NewServer(grpcOpts...)
 
-	nodeId := opts.Raft.GetUint64("idx")
+	nodeId := opts.raft.GetUint64("idx")
 	rc := pb.RaftContext{
 		Id:        nodeId,
 		Addr:      x.WorkerConfig.MyAddr,
 		Group:     0,
-		IsLearner: opts.Raft.GetBool("learner"),
+		IsLearner: opts.raft.GetBool("learner"),
 	}
 	m := conn.NewNode(&rc, store, opts.tlsClientConfig)
 
@@ -200,7 +200,9 @@ func (st *state) serveGRPC(l net.Listener, store *raftwal.DiskStorage) {
 }
 
 func run() {
-	if Zero.Conf.GetBool("enable_sentry") {
+	telemetry := z.NewSuperFlag(Zero.Conf.GetString("telemetry")).MergeAndCheckDefault(
+		x.TelemetryDefaults)
+	if telemetry.GetBool("sentry") {
 		x.InitSentry(enc.EeBuild)
 		defer x.FlushSentry()
 		x.ConfigureSentryScope("zero")
@@ -212,12 +214,14 @@ func run() {
 	tlsConf, err := x.LoadClientTLSConfigForInternalPort(Zero.Conf)
 	x.Check(err)
 
-	raft := z.NewSuperFlag(Zero.Conf.GetString("raft")).MergeAndCheckDefault(raftDefaults)
+	raft := z.NewSuperFlag(Zero.Conf.GetString("raft")).MergeAndCheckDefault(
+		raftDefaults)
 	conf := audit.GetAuditConf(Zero.Conf.GetString("audit"))
 	opts = options{
+		telemetry:         telemetry,
+		raft:              raft,
 		bindall:           Zero.Conf.GetBool("bindall"),
 		portOffset:        Zero.Conf.GetInt("port_offset"),
-		Raft:              raft,
 		numReplicas:       Zero.Conf.GetInt("replicas"),
 		peer:              Zero.Conf.GetString("peer"),
 		w:                 Zero.Conf.GetString("wal"),
@@ -270,7 +274,7 @@ func run() {
 		x.WorkerConfig.MyAddr = fmt.Sprintf("localhost:%d", x.PortZeroGrpc+opts.portOffset)
 	}
 
-	nodeId := opts.Raft.GetUint64("idx")
+	nodeId := opts.raft.GetUint64("idx")
 	if nodeId == 0 {
 		log.Fatalf("ERROR: raft.idx flag cannot be 0. Please set idx to a unique positive integer.")
 	}
@@ -308,7 +312,7 @@ func run() {
 	// This must be here. It does not work if placed before Grpc init.
 	x.Check(st.node.initAndStartNode())
 
-	if Zero.Conf.GetBool("telemetry") {
+	if opts.telemetry.GetBool("reports") {
 		go st.zero.periodicallyPostTelemetry()
 	}
 

diff --git a/ee/audit/audit_ee.go b/ee/audit/audit_ee.go
@@ -59,7 +59,7 @@ type auditLogger struct {
 }
 
 func GetAuditConf(conf string) *x.LoggerConf {
-	if conf == "" {
+	if conf == "" || conf == worker.AuditDefaults {
 		return nil
 	}
 	auditFlag := z.NewSuperFlag(conf).MergeAndCheckDefault(worker.AuditDefaults)