Skip to content

opt(predMove): iterate Phase I till there is major data to move #7792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 11, 2021

Conversation

NamanJain8
Copy link
Contributor

@NamanJain8 NamanJain8 commented May 6, 2021

Discuss post: https://discuss.dgraph.io/t/13888
Relates to DGRAPH-3321

This PR adds optimization to predicate move by staying in Phase I, it took more than 20 mins.


This change is Reviewable

@NamanJain8 NamanJain8 marked this pull request as draft May 6, 2021 15:20
@NamanJain8 NamanJain8 marked this pull request as ready for review May 6, 2021 16:06
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: Just ensure that it's correct.

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @NamanJain8 and @vvbalaji-dgraph)


dgraph/cmd/zero/tablet.go, line 180 at r1 (raw file):

	var sinceTs uint64
	counter := 0
	phaseI := func() (*api.Payload, error) {

nonBlockingMove


dgraph/cmd/zero/tablet.go, line 206 at r1 (raw file):

		if err != nil {
			return err
		}

glog.Infof(recvCount was x)


dgraph/cmd/zero/tablet.go, line 207 at r1 (raw file):

			return err
		}
		if recvCount < phaseOneThreshold || counter > 5 {

3

also glog.Infof saying will redo phase 1. Also, put all this in span too.

@NamanJain8 NamanJain8 force-pushed the naman/opt-pred-move branch from d8be1c5 to 8249ad7 Compare May 10, 2021 16:04
@NamanJain8 NamanJain8 merged commit 5549acd into master May 11, 2021
@NamanJain8 NamanJain8 deleted the naman/opt-pred-move branch May 11, 2021 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants