Skip to content
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

Support comma separated list of zero addresses in alpha #5116

Merged
merged 3 commits into from
Apr 8, 2020

Conversation

jarifibrahim
Copy link
Contributor

@jarifibrahim jarifibrahim commented Apr 6, 2020

Description.

Currently, alpha supports specifying only a single zero instance address via the --zero flag in dgraph alpha command. This PR proposes supporting a comma-separated list of zero addresses in dgraph alpha so that any other address can be used from the list of zero addresses if one of them is unavailable.

One of the zero addresses will be randomly picked from the list and we'll try to establish a connection to it. If it fails, we'll pick the next zero address.

The dgraph bulk/live command accepts only a single zero address. The comma-separated list is only for dgraph alpha command.

How to test this PR

  1. Start 3 zero and an alpha instance.
  2. Provide alpha with multiple zero addresses
  3. Kill the zero instance alpha was connected to. Alpha would automatically rectify the failure and connect to another zero instance.
  4. Kill the running alpha instance
  5. Try to start alpha again (ensure you have at least one zero running)
  6. Alpha should be able to start (Without this PR, alpha wouldn't start at this point because the zero specified via zero flag is unavailable).

GitHub Issue or Jira number.

https://dgraph.atlassian.net/browse/DGRAPH-1132
#4949

Other components or 3rd party tools affected (or regression areas).

Affected releases.

20.03.1, 20.07

Changelog tags.

Support command separated list of zero addresses in alpha

Please indicate if this is a breaking change.

No

Please indicate if this is an enterprise feature.

No

Please indicate if documentation needs to be updated.

https://dgraph.atlassian.net/browse/DGRAPH-1203

Please indicate if end to end testing is needed.

No

Fixes #4949


This change is Reviewable

Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jarifibrahim, @manishrjain, and @martinmr)


worker/groups.go, line 93 at r1 (raw file):

	for _, zeroAddr := range x.WorkerConfig.ZeroAddr {
		x.AssertTruef(zeroAddr != x.WorkerConfig.MyAddr,
			"Dgraph Zero address and Dgraph address (IP:Port) can't be the same.")

The zeroAddr that is same should be printed here now that we give more than one.


x/config.go, line 57 at r1 (raw file):

	MyAddr string
	// ZeroAddr stores the list of address:port for the zero instances associated with this alpha.
	// Alpha would be communicate via only one zero address from the list. All

would communicate

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: Got a comment.

Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @jarifibrahim and @martinmr)


worker/groups.go, line 653 at r1 (raw file):

	delay := connBaseDelay
	maxHalfDelay := time.Second
	randSrc := rand.New(rand.NewSource(time.Now().UnixNano()))

Instead of random, let's use round-robin. Use a counter for that.


worker/groups.go, line 654 at r1 (raw file):

	maxHalfDelay := time.Second
	randSrc := rand.New(rand.NewSource(time.Now().UnixNano()))
	for { // Keep on retrying. See: https://github.com/dgraph-io/dgraph/issues/2289

for i := 0; ; i++ {


worker/groups.go, line 662 at r1 (raw file):

		zAddrList := x.WorkerConfig.ZeroAddr
		// Pick a random zero address.
		addr := zAddrList[randSrc.Intn(len(zAddrList))]

zAddrList[i%num]

Copy link
Contributor Author

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 5 files reviewed, 5 unresolved discussions (waiting on @manishrjain, @martinmr, and @pawanrawal)


worker/groups.go, line 93 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

The zeroAddr that is same should be printed here now that we give more than one.

Done.


worker/groups.go, line 653 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Instead of random, let's use round-robin. Use a counter for that.

Done.


worker/groups.go, line 654 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

for i := 0; ; i++ {

Done.


worker/groups.go, line 662 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

zAddrList[i%num]

Done.


x/config.go, line 57 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

would communicate

Done.

@jarifibrahim jarifibrahim merged commit 82adb16 into master Apr 8, 2020
@jarifibrahim jarifibrahim deleted the ibrahim/zero-multiple-address branch April 8, 2020 11:06
jarifibrahim pushed a commit that referenced this pull request Apr 21, 2020
Currently, alpha supports specifying only a single zero instance
address via the `--zero` flag in `dgraph alpha` command. This commit
adds supports for a comma-separated list of `zero` addresses in
`dgraph alpha` so that any zero address can be used from the list
of zero addresses if one of them is unavailable.

We'll pick zero addresses starting from the first one in the list and
try to establish a connection to it. If it fails, we'll pick
the next zero address.

The `dgraph bulk/live` command accepts only a single zero address.
The comma-separated list is only for `dgraph alpha` command.

(cherry picked from commit 82adb16)
jarifibrahim pushed a commit that referenced this pull request Apr 22, 2020
Currently, alpha supports specifying only a single zero instance
address via the `--zero` flag in `dgraph alpha` command. This commit
adds supports for a comma-separated list of `zero` addresses in
`dgraph alpha` so that any zero address can be used from the list
of zero addresses if one of them is unavailable.

We'll pick zero addresses starting from the first one in the list and
try to establish a connection to it. If it fails, we'll pick
the next zero address.

The `dgraph bulk/live` command accepts only a single zero address.
The comma-separated list is only for `dgraph alpha` command.

(cherry picked from commit 82adb16)
danielmai pushed a commit that referenced this pull request Apr 24, 2020
Currently, alpha supports specifying only a single zero instance
address via the `--zero` flag in `dgraph alpha` command. This commit
adds supports for a comma-separated list of `zero` addresses in
`dgraph alpha` so that any zero address can be used from the list
of zero addresses if one of them is unavailable.

We'll pick zero addresses starting from the first one in the list and 
try to establish a connection to it. If it fails, we'll pick
the next zero address.

The `dgraph bulk/live` command accepts only a single zero address.
The comma-separated list is only for `dgraph alpha` command.
danielmai pushed a commit that referenced this pull request Apr 24, 2020
Currently, alpha supports specifying only a single zero instance
address via the `--zero` flag in `dgraph alpha` command. This commit
adds supports for a comma-separated list of `zero` addresses in
`dgraph alpha` so that any zero address can be used from the list
of zero addresses if one of them is unavailable.

We'll pick zero addresses starting from the first one in the list and 
try to establish a connection to it. If it fails, we'll pick
the next zero address.

The `dgraph bulk/live` command accepts only a single zero address.
The comma-separated list is only for `dgraph alpha` command.
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 18, 2020
Currently, alpha supports specifying only a single zero instance
address via the `--zero` flag in `dgraph alpha` command. This commit
adds supports for a comma-separated list of `zero` addresses in
`dgraph alpha` so that any zero address can be used from the list
of zero addresses if one of them is unavailable.

We'll pick zero addresses starting from the first one in the list and 
try to establish a connection to it. If it fails, we'll pick
the next zero address.

The `dgraph bulk/live` command accepts only a single zero address.
The comma-separated list is only for `dgraph alpha` command.
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.

Allow comma separated list of values in the alpha's --zero option
4 participants