Optimize computing reverse reindexing #4755
Conversation
mangalaman93
requested review from
martinmr,
manishrjain,
pawanrawal and
ashish-goswami
mangalaman93
force-pushed
the
aman/optimization
branch
from
80fdaad
to
6569a26
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @mangalaman93, @manishrjain, and @pawanrawal)
dgraph/cmd/alpha/reindex_test.go, line 140 at r1 (raw file):
}` _, err := mutationWithTs(m1, "application/rdf", false, true, 0) require.NoError(t, err)
just for completeness sake, do the query here and verify there's an error due to the lack of an index.
posting/index.go, line 1010 at r1 (raw file):
// we only need to build reverse index here. // We will update the reverse count index separately.
This can be one line.
mangalaman93
force-pushed
the
aman/optimization
branch
from
6569a26
to
728994b
Compare
We used to compute reverse count indexes twice while reindexing. First, while computing reverse edges, and then later while explicitly computing reverse count index. Now, we use a different function while reindexing reverse edges and do not compute the reverse count indexes.
mangalaman93
force-pushed
the
aman/optimization
branch
from
728994b
to
1e92791
Compare
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @manishrjain, @martinmr, and @pawanrawal)
dgraph/cmd/alpha/reindex_test.go, line 140 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
just for completeness sake, do the query here and verify there's an error due to the lack of an index.
There are already tests that do this. In this test, I want to make sure that reindexing path as well as mutation both make correct changes to storage.
posting/index.go, line 1010 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
// we only need to build reverse index here. // We will update the reverse count index separately.This can be one line.
It's fine, doesn't seem like a big deal to me.
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @martinmr, and @pawanrawal)
We used to compute reverse count indexes twice while reindexing. First, while computing reverse edges, and then later while explicitly computing reverse count index. Now, we use a different function while reindexing reverse edges and do not compute the reverse count indexes there any more.
We used to compute reverse count indexes twice while reindexing. First, while computing reverse edges, and then later while explicitly computing reverse count index. Now, we use a different function while reindexing reverse edges and do not compute the reverse count indexes there any more.
#6748) Fixes the test OverwriteUidPredicatesReverse added in the previous PR #6746. When updating a single uid predicate with a reverse index, the existing entry in the reverse index should be deleted first. Fixes DGRAPH-1738 (cherry picked from commit 5b70fe8) * Remove context arguments. * Optimize computing reverse reindexing (#4755) We used to compute reverse count indexes twice while reindexing. First, while computing reverse edges, and then later while explicitly computing reverse count index. Now, we use a different function while reindexing reverse edges and do not compute the reverse count indexes there any more. * fix(Dgraph): update reverse index when updating single UID predicates. (#5868) When updating a single uid predicate with a reverse index, the existing entry in the reverse index should be deleted first. Fixes DGRAPH-1738 Co-authored-by: Martin Martinez Rivera <martinmr@dgraph.io> Co-authored-by: Aman Mangal <aman@dgraph.io>
We used to compute reverse count indexes twice while reindexing.
First, while computing reverse edges, and then later while explicitly
computing reverse count index. Now, we use a different function
while reindexing reverse edges and do not compute the reverse
count indexes.
This change is
Docs Preview: