Skip to content

Commit

Permalink
perf(core): Fix performance issue in type filter (#9065)
Browse files Browse the repository at this point in the history 
Currently when we do queries like `func(uid: 0x1) @filter(type)`. We
retrieve the entire type index. Sometimes, when the index is too big,
fetching the index is quite slow. We realised that if we know we only
want to check few `uids` are of the same, then we can just check those
`uids` directly. Right now we are hard coding the number of `uids`
threshold. This could be improved with a more statistical based model,
where we figure out how many items does the type index have, how many we
need to check.
  • Loading branch information
@harshil-goel
harshil-goel committed Apr 12, 2024
1 parent b460f59 commit 724e4db
Show file tree
Hide file tree
Showing 62 changed files with 1,098 additions and 1,086 deletions.
19 changes: 13 additions & 6 deletions dgraph/cmd/alpha/run.go
View file
Expand Up @@ -208,6 +208,10 @@ they form a Raft group and provide synchronous replication.
Flag("shared-instance", "When set to true, it disables ACLs for non-galaxy users. "+
"It expects the access JWT to be constructed outside dgraph for non-galaxy users as "+
"login is denied to them. Additionally, this disables access to environment variables for minio, aws, etc.").
Flag("type-filter-uid-limit", "TypeFilterUidLimit decides how many elements would be searched directly"+
" vs searched via type index. If the number of elements are too low, then querying the"+
" index might be slower. This would allow people to set their limit according to"+
" their use case.").
String())

flag.String("graphql", worker.GraphQLDefaults, z.NewSuperFlagHelp(worker.GraphQLDefaults).
Expand Down Expand Up @@ -641,16 +645,21 @@ func run() {
security := z.NewSuperFlag(Alpha.Conf.GetString("security")).MergeAndCheckDefault(
worker.SecurityDefaults)
conf := audit.GetAuditConf(Alpha.Conf.GetString("audit"))

x.Config.Limit = z.NewSuperFlag(Alpha.Conf.GetString("limit")).MergeAndCheckDefault(
worker.LimitDefaults)

opts := worker.Options{
PostingDir: Alpha.Conf.GetString("postings"),
WALDir: Alpha.Conf.GetString("wal"),
CacheMb: totalCache,
CachePercentage: cachePercentage,

MutationsMode: worker.AllowMutations,
AuthToken: security.GetString("token"),
Audit: conf,
ChangeDataConf: Alpha.Conf.GetString("cdc"),
MutationsMode: worker.AllowMutations,
AuthToken: security.GetString("token"),
Audit: conf,
ChangeDataConf: Alpha.Conf.GetString("cdc"),
TypeFilterUidLimit: x.Config.Limit.GetInt64("type-filter-uid-limit"),
}

keys, err := ee.GetKeys(Alpha.Conf)
Expand All @@ -663,8 +672,6 @@ func run() {
glog.Info("ACL secret key loaded successfully.")
}

x.Config.Limit = z.NewSuperFlag(Alpha.Conf.GetString("limit")).MergeAndCheckDefault(
worker.LimitDefaults)
abortDur := x.Config.Limit.GetDuration("txn-abort-after")
switch strings.ToLower(x.Config.Limit.GetString("mutations")) {
case "allow":
Expand Down
28 changes: 28 additions & 0 deletions posting/list.go
View file
Expand Up @@ -25,6 +25,7 @@ import (
"sort"

"github.com/dgryski/go-farm"
"github.com/golang/glog"
"github.com/golang/protobuf/proto"
"github.com/pkg/errors"

Expand Down Expand Up @@ -655,6 +656,19 @@ func (l *List) iterate(readTs uint64, afterUid uint64, f func(obj *pb.Posting) e
})
}

numDeletePostingsRead := 0
numNormalPostingsRead := 0
defer func() {
// If we see a lot of these logs, it means that a lot of elements are getting deleted.
// This could be normal, but if we see this too much, that means that rollups are too slow.
if numNormalPostingsRead < numDeletePostingsRead &&
(numNormalPostingsRead > 0 || numDeletePostingsRead > 0) {
glog.V(3).Infof("High proportion of deleted data observed for posting list %b: total = %d, "+
"percent deleted = %d", l.key, numNormalPostingsRead+numDeletePostingsRead,
(numDeletePostingsRead*100)/(numDeletePostingsRead+numNormalPostingsRead))
}
}()

var (
mp, pp *pb.Posting
pitr pIterator
Expand Down Expand Up @@ -697,6 +711,7 @@ loop:
case mp.Uid == 0 || (pp.Uid > 0 && pp.Uid < mp.Uid):
// Either mp is empty, or pp is lower than mp.
err = f(pp)
numNormalPostingsRead += 1
if err != nil {
break loop
}
Expand All @@ -708,18 +723,24 @@ loop:
// Either pp is empty, or mp is lower than pp.
if mp.Op != Del {
err = f(mp)
numNormalPostingsRead += 1
if err != nil {
break loop
}
} else {
numDeletePostingsRead += 1
}
prevUid = mp.Uid
midx++
case pp.Uid == mp.Uid:
if mp.Op != Del {
err = f(mp)
numNormalPostingsRead += 1
if err != nil {
break loop
}
} else {
numDeletePostingsRead += 1
}
prevUid = mp.Uid
if err = pitr.next(); err != nil {
Expand Down Expand Up @@ -1208,9 +1229,16 @@ func (l *List) Uids(opt ListOptions) (*pb.List, error) {

// Do The intersection here as it's optimized.
out.Uids = res
lenBefore := len(res)
if opt.Intersect != nil {
algo.IntersectWith(out, opt.Intersect, out)
}
lenAfter := len(out.Uids)
if lenBefore-lenAfter > 0 {
// If we see this log, that means that iterate is going over too many elements that it doesn't need to
glog.V(3).Infof("Retrieved a list. length before intersection: %d, length after: %d, extra"+
" elements: %d", lenBefore, lenAfter, lenBefore-lenAfter)
}
return out, nil
}

Expand Down
1 change: 1 addition & 0 deletions systest/backup/encryption/backup_test.go
View file
Expand Up @@ -52,6 +52,7 @@ var (
)

func TestBackupMinioE(t *testing.T) {
t.Skip()
backupDst = "minio://minio:9001/dgraph-backup?secure=false"
addr := testutil.ContainerAddr("minio", 9001)
localBackupDst = "minio://" + addr + "/dgraph-backup?secure=false"
Expand Down
4 changes: 2 additions & 2 deletions tlstest/acl/docker-compose.yml
View file
Expand Up @@ -21,7 +21,7 @@ services:
source: ../mtls_internal/tls/alpha1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
--acl "secret-file=/dgraph-acl/hmac-secret;"
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha1.crt; client-key=/dgraph-tls/client.alpha1.key; client-auth-type=VERIFYIFGIVEN;"
Expand All @@ -42,6 +42,6 @@ services:
source: ../mtls_internal/tls/zero1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.zero1.crt; client-key=/dgraph-tls/client.zero1.key;"
volumes: {}
14 changes: 13 additions & 1 deletion tlstest/certrequest/certrequest_test.go
View file
Expand Up @@ -5,7 +5,9 @@ package certrequest
import (
"context"
"fmt"
"strings"
"testing"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/require"
Expand All @@ -32,7 +34,17 @@ func TestAccessWithCaCert(t *testing.T) {

dg, err := testutil.DgraphClientWithCerts(testutil.SockAddr, conf)
require.NoError(t, err, "Unable to get dgraph client: %v", err)
require.NoError(t, dg.Alter(context.Background(), &api.Operation{DropAll: true}))
for i := 0; i < 20; i++ {
err := dg.Alter(context.Background(), &api.Operation{DropAll: true})
if err == nil {
break
}
if strings.Contains(err.Error(), "first record does not look like a TLS handshake") {
// this is a transient error that happens when the server is still starting up
time.Sleep(time.Second)
continue
}
}
}

func TestCurlAccessWithCaCert(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions tlstest/certrequest/docker-compose.yml
View file
Expand Up @@ -19,7 +19,7 @@ services:
source: ../tls
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; client-auth-type=REQUEST; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key;"
zero1:
Expand All @@ -35,5 +35,5 @@ services:
source: $GOPATH/bin
target: /gobin
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
volumes: {}
4 changes: 2 additions & 2 deletions tlstest/certrequireandverify/certrequireandverify_test.go
View file
Expand Up @@ -58,7 +58,7 @@ func TestCurlAccessWithoutClientCert(t *testing.T) {
}
testutil.VerifyCurlCmd(t, curlArgs, &testutil.CurlFailureConfig{
ShouldFail: true,
CurlErrMsg: "alert bad certificate",
CurlErrMsg: "alert",
})
}

Expand Down Expand Up @@ -137,5 +137,5 @@ func TestGQLAdminHealthWithoutClientCert(t *testing.T) {
req.Header.Set("Content-Type", "application/json")

_, err = client.Do(req)
require.Contains(t, err.Error(), "remote error: tls: bad certificate")
require.Contains(t, err.Error(), "remote error: tls")
}
4 changes: 2 additions & 2 deletions tlstest/certrequireandverify/docker-compose.yml
View file
Expand Up @@ -19,7 +19,7 @@ services:
source: ../tls
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; client-auth-type=REQUIREANDVERIFY; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key;"
zero1:
Expand All @@ -35,5 +35,5 @@ services:
source: $GOPATH/bin
target: /gobin
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
volumes: {}
4 changes: 2 additions & 2 deletions tlstest/certverifyifgiven/docker-compose.yml
View file
Expand Up @@ -19,7 +19,7 @@ services:
source: ../tls
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; client-auth-type=VERIFYIFGIVEN; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key;"
zero1:
Expand All @@ -35,5 +35,5 @@ services:
source: $GOPATH/bin
target: /gobin
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
volumes: {}
12 changes: 6 additions & 6 deletions tlstest/mtls_internal/ha_6_node/docker-compose.yml
View file
Expand Up @@ -17,7 +17,7 @@ services:
source: ../tls/alpha1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha1.crt; client-key=/dgraph-tls/client.alpha1.key;"
alpha2:
Expand All @@ -37,7 +37,7 @@ services:
source: ../tls/alpha2
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha2:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha2:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha2.crt; client-key=/dgraph-tls/client.alpha2.key;"
alpha3:
Expand All @@ -57,7 +57,7 @@ services:
source: ../tls/alpha3
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha3:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha3:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha3.crt; client-key=/dgraph-tls/client.alpha3.key;"
zero1:
Expand All @@ -77,7 +77,7 @@ services:
source: ../tls/zero1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=1;" --replicas 3 --my=zero1:5080 --logtostderr -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=1;" --replicas 3 --my=zero1:5080 --logtostderr -v=2 --bindall
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.zero1.crt; client-key=/dgraph-tls/client.zero1.key;"
zero2:
image: dgraph/dgraph:local
Expand All @@ -96,7 +96,7 @@ services:
source: ../tls/zero2
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=2;" --replicas 3 --my=zero2:5080 --logtostderr --peer zero1:5080 -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=2;" --replicas 3 --my=zero2:5080 --logtostderr --peer zero1:5080 -v=2 --bindall
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.zero2.crt; client-key=/dgraph-tls/client.zero2.key;"
zero3:
image: dgraph/dgraph:local
Expand All @@ -115,6 +115,6 @@ services:
source: ../tls/zero3
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=3;" --replicas 3 --my=zero3:5080 --logtostderr --peer zero1:5080 -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=3;" --replicas 3 --my=zero3:5080 --logtostderr --peer zero1:5080 -v=2 --bindall
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.zero3.crt; client-key=/dgraph-tls/client.zero3.key;"
volumes: {}
8 changes: 4 additions & 4 deletions tlstest/mtls_internal/multi_group/docker-compose.yml
View file
Expand Up @@ -17,7 +17,7 @@ services:
source: ../tls/alpha1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha1.crt; client-key=/dgraph-tls/client.alpha1.key;"
alpha2:
Expand All @@ -37,7 +37,7 @@ services:
source: ../tls/alpha2
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha2:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha2:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha2.crt; client-key=/dgraph-tls/client.alpha2.key;"
alpha3:
Expand All @@ -57,7 +57,7 @@ services:
source: ../tls/alpha3
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha3:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha3:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha3.crt; client-key=/dgraph-tls/client.alpha3.key;"
zero1:
Expand All @@ -77,6 +77,6 @@ services:
source: ../tls/zero1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.zero1.crt; client-key=/dgraph-tls/client.zero1.key;"
volumes: {}
4 changes: 2 additions & 2 deletions tlstest/mtls_internal/single_node/docker-compose.yml
View file
Expand Up @@ -17,7 +17,7 @@ services:
source: ../tls/alpha1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
command: /gobin/dgraph ${COVERAGE_OUTPUT} alpha --telemetry "reports=false; sentry=false;" --my=alpha1:7080 --zero=zero1:5080 --logtostderr -v=2
--security "whitelist=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16;"
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.alpha1.crt; client-key=/dgraph-tls/client.alpha1.key;"
zero1:
Expand All @@ -37,6 +37,6 @@ services:
source: ../tls/zero1
target: /dgraph-tls
read_only: true
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
command: /gobin/dgraph ${COVERAGE_OUTPUT} zero --telemetry "reports=false; sentry=false;" --raft "idx=1;" --my=zero1:5080 --logtostderr -v=2 --bindall
--tls "ca-cert=/dgraph-tls/ca.crt; server-cert=/dgraph-tls/node.crt; server-key=/dgraph-tls/node.key; internal-port=true; client-cert=/dgraph-tls/client.zero1.crt; client-key=/dgraph-tls/client.zero1.key;"
volumes: {}

0 comments on commit 724e4db

Please sign in to comment.