Skip to content

Commit

Permalink
Add LogRequest variable to config input (#5197) (#7085)
Browse files Browse the repository at this point in the history
* Add LogRequest variable to config input (#5197)

Fixes: DGRAPH-1123

This PR adds support for logRequest config param. logRequest can have true/false values. True
value of logRequest enables logging of all requests coming to alphas. False value of logRequest
disables it.
Users will be able to dynamically change this param using graphql admin endpoint.
Sample query:

mutation {
  config(input: {logRequest: true}){
    response {
      code
      message
    }
  }
}
Internally we have added one more field in WorkerOptions(WorkerConfig) called as LogRequest
of int32 type. Value of this field is checked atomically upon every request arrival. If value of
LogRequest is 1, it results in logging of requests.


Co-authored-by: Ashish Goswami <ashish@dgraph.io>
(cherry picked from commit 449c06b)

* fix(GraphQL): don't update cacheMb if not specified by user (GRAPHQL-888) (#7103)

Fixes GRAPHQL-888.
Previously, if you ran this request:
```
$ curl -H "Content-Type: application/json" http://localhost:8080/admin -d '{"query": "mutation {config(input: {logRequest: false}){response {code message}}}"}'
```
Alpha logs would also print this:
```
I1205 22:22:51.684693 2681396 middlewares.go:178] GraphQL admin mutation. Name =  config
I1205 22:22:51.684724 2681396 config.go:38] Got config update through GraphQL admin API
I1205 22:22:51.684810 2681396 worker.go:138] Updating cacheMb to 0
```
Indicating that cacheMb was also updated, even it wasn't specified in the request.

This PR fixes this issue.

(cherry picked from commit c03c327)

# Conflicts:
#	graphql/admin/config.go

Co-authored-by: Animesh Chandra Pathak <animesh@dgraph.io>
Co-authored-by: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com>
  • Loading branch information
3 people committed Dec 10, 2020
1 parent 91387c2 commit 07b87c2
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 5 deletions.
2 changes: 1 addition & 1 deletion edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ func (s *Server) Query(ctx context.Context, req *api.Request) (*api.Response, er

func (s *Server) doQuery(ctx context.Context, req *api.Request, doAuth AuthMode) (
resp *api.Response, rerr error) {
if glog.V(3) {
if bool(glog.V(3)) || worker.LogRequestEnabled() {
glog.Infof("Got a query: %+v", req)
}
isGraphQL, _ := ctx.Value(IsGraphql).(bool)
Expand Down
6 changes: 6 additions & 0 deletions graphql/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ const (
more than specified here. (default -1 means no set limit)
"""
lruMb: Float
"""
True value of logRequest enables logging of all the requests coming to alphas.
False value of logRequest disables above.
"""
logRequest: Boolean
}
type ConfigPayload {
Expand Down
19 changes: 15 additions & 4 deletions graphql/admin/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ import (
)

type configInput struct {
LruMB float64
LruMB *float64
// LogRequest is used to update WorkerOptions.LogRequest. true value of LogRequest enables
// logging of all requests coming to alphas. LogRequest type has been kept as *bool instead of
// bool to avoid updating WorkerOptions.LogRequest when it has default value of false.
LogRequest *bool
}

func resolveConfig(ctx context.Context, m schema.Mutation) (*resolve.Resolved, bool) {
Expand All @@ -38,9 +42,16 @@ func resolveConfig(ctx context.Context, m schema.Mutation) (*resolve.Resolved, b
return emptyResult(m, err), false
}

err = worker.UpdateLruMb(input.LruMB)
if err != nil {
return emptyResult(m, err), false
// update LruMB only when it is specified by user
if input.LruMB != nil {
if err = worker.UpdateLruMb(*input.LruMB); err != nil {
return emptyResult(m, err), false
}
}

// input.LogRequest will be nil, when it is not specified explicitly in config request.
if input.LogRequest != nil {
worker.UpdateLogRequest(*input.LogRequest)
}

return &resolve.Resolved{
Expand Down
16 changes: 16 additions & 0 deletions worker/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"math"
"net"
"sync"
"sync/atomic"

"github.com/dgraph-io/badger/v2"
badgerpb "github.com/dgraph-io/badger/v2/pb"
Expand Down Expand Up @@ -139,3 +140,18 @@ func UpdateLruMb(memoryMB float64) error {
posting.Config.Unlock()
return nil
}

// UpdateLogRequest updates value of x.WorkerConfig.LogRequest.
func UpdateLogRequest(val bool) {
if val {
atomic.StoreInt32(&x.WorkerConfig.LogRequest, 1)
return
}

atomic.StoreInt32(&x.WorkerConfig.LogRequest, 0)
}

// LogRequestEnabled returns true if logging of requests is enabled otherwise false.
func LogRequestEnabled() bool {
return atomic.LoadInt32(&x.WorkerConfig.LogRequest) > 0
}
5 changes: 5 additions & 0 deletions x/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ type WorkerOptions struct {
LudicrousMode bool
// BadgerKeyFile is the file containing the key used for encryption. Enterprise only feature.
BadgerKeyFile string
// LogRequest indicates whether alpha should log all query/mutation requests coming to it.
// Ideally LogRequest should be a bool value. But we are reading it using atomics across
// queries hence it has been kept as int32. LogRequest value 1 enables logging of requests
// coming to alphas and 0 disables it.
LogRequest int32
}

// WorkerConfig stores the global instance of the worker package's options.
Expand Down

0 comments on commit 07b87c2

Please sign in to comment.