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

Issues/GitHub #1266

Merged
merged 10 commits into from
Jul 28, 2017
Merged

Issues/GitHub #1266

merged 10 commits into from
Jul 28, 2017

Conversation

janardhan1993
Copy link
Contributor

@janardhan1993 janardhan1993 commented Jul 27, 2017

This change is Reviewable

@manishrjain
Copy link
Contributor

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


cmd/dgraph/main.go, line 716 at r1 (raw file):

			select {
			case _, ok := <-sdCh:
				if ok {

No need for ok.


cmd/dgraph/main.go, line 717 at r1 (raw file):

			case _, ok := <-sdCh:
				if ok {
					numShutDownSig++

After this line, print "Caught ctrl+c. Terminating now (this may take a few seconds)... "


cmd/dgraph/main.go, line 721 at r1 (raw file):

						shutdownServer()
					} else if numShutDownSig == 3 {
						os.Exit(1)

"Signaled thrice. Aborting!"


Comments from Reviewable

@janardhan1993
Copy link
Contributor Author

Review status: 1 of 4 files reviewed at latest revision, 3 unresolved discussions.


cmd/dgraph/main.go, line 716 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

No need for ok.

We need ok, i checked after a channel is closed this will get triggered and even on clean exit we would end up printing aborted.


cmd/dgraph/main.go, line 717 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

After this line, print "Caught ctrl+c. Terminating now (this may take a few seconds)... "

Done.


cmd/dgraph/main.go, line 721 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

"Signaled thrice. Aborting!"

Done.


Comments from Reviewable

@manishrjain
Copy link
Contributor

:lgtm:


Reviewed 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks broke.


cmd/dgraph/main.go, line 716 at r2 (raw file):

			select {
			case _, ok := <-sdCh:
				if ok {

if !ok { // channel is closed.
return
}


cmd/dgraph/main.go, line 718 at r2 (raw file):

				if ok {
					numShutDownSig++
					fmt.Printf("Caught Ctrl-C. Terminating now (this may take a few seconds)...\n")

Println


cmd/dgraph/main.go, line 722 at r2 (raw file):

						shutdownServer()
					} else if numShutDownSig == 3 {
						fmt.Printf("Signaled thrice. Aborting!\n")

Println


worker/draft.go, line 245 at r2 (raw file):

			HeartbeatTick:   1,
			Storage:         store,
			MaxSizePerMsg:   128 << 10,

256 << 10.


Comments from Reviewable

@janardhan1993
Copy link
Contributor Author

Review status: 3 of 5 files reviewed at latest revision, 4 unresolved discussions.


cmd/dgraph/main.go, line 716 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

if !ok { // channel is closed.
return
}

Done.


cmd/dgraph/main.go, line 718 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Println

Done.


cmd/dgraph/main.go, line 722 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Println

Done.


Comments from Reviewable

@janardhan1993
Copy link
Contributor Author

Review status: 3 of 5 files reviewed at latest revision, 4 unresolved discussions.


worker/draft.go, line 245 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

256 << 10.

Done.


Comments from Reviewable

@janardhan1993 janardhan1993 merged commit 7eecb1f into master Jul 28, 2017
@janardhan1993 janardhan1993 deleted the issues/github branch July 28, 2017 01:57
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.

2 participants