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

fix(race): fixing race condition with closer use + assign uids + bgindex test fix #7266

Merged
merged 1 commit into from
Jan 11, 2021

Conversation

aman-bansal
Copy link
Contributor

@aman-bansal aman-bansal commented Jan 8, 2021

This change is Reviewable

@aman-bansal aman-bansal marked this pull request as draft January 8, 2021 19:57
@aman-bansal aman-bansal force-pushed the aman/fix_alpha_race_conditions branch from c8b0fee to a0d8834 Compare January 8, 2021 20:27
@aman-bansal aman-bansal marked this pull request as ready for review January 11, 2021 05:57
Copy link
Contributor

@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.

Reduce the number of changes.

@aman-bansal aman-bansal force-pushed the aman/fix_alpha_race_conditions branch from a0d8834 to 970c21c Compare January 11, 2021 08:04
@@ -196,7 +196,7 @@ func (s *Server) AssignUids(ctx context.Context, num *pb.Num) (*pb.AssignedIds,

select {
case <-ctx.Done():
return reply, ctx.Err()
return &emptyAssignedIds, ctx.Err()
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need to change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a race condition with the use of reply. This happens when ctx.Done was called but lease is still working to assign uids

@@ -19,109 +19,8 @@ services:
source: ../../ee/acl/hmac-secret
target: /secret/hmac
read_only: true
command: /gobin/dgraph alpha --my=alpha1:7080 --zero=zero1:5080,zero2:5080,zero3:5080
Copy link
Contributor

Choose a reason for hiding this comment

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

So this PR has 3 changes?

  1. closer fix
  2. Assign UID fix
  3. bgindex fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this PR has above three fix. let me change the description also

@aman-bansal aman-bansal changed the title fix(race): fixing race condition with closer use + assign uids fix(race): fixing race condition with closer use + assign uids + bgindex test fix Jan 11, 2021
@aman-bansal aman-bansal merged commit 65be0bd into master Jan 11, 2021
@aman-bansal aman-bansal deleted the aman/fix_alpha_race_conditions branch January 12, 2021 06:54
aman-bansal added a commit that referenced this pull request Jan 12, 2021
aman-bansal added a commit that referenced this pull request Jan 12, 2021
* fix(race): fixing race condition in localcache (#7140)

* fixing race condition in localcache

* name refactor

* fixing race in pick postings + fixing race log in t.go (#7149)

* fixing race condition in process subgraph (#7156)

* fixing race condition in disk storage sync and save (#7263)

* fixing bgindex + fix alpha z.closer (#7266)
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