Skip to content

Conversation

@jarifibrahim
Copy link
Contributor

@jarifibrahim jarifibrahim commented Aug 8, 2019

The TestGetMore test fails with race condition in updateDiscardStats
function.
See https://teamcity.dgraph.io/viewLog.html?buildId=19032&buildTypeId=Badger_UnitTests


This change is Reviewable

The TestGetMore test fails with race condition in updateDiscardStats
function.
See https://teamcity.dgraph.io/viewLog.html?buildId=19032&buildTypeId=Badger_UnitTests
@jarifibrahim jarifibrahim requested a review from a team August 8, 2019 06:45
Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ A review job has been created and sent to the PullRequest network.


@jarifibrahim you can click here to see the review status or cancel the code review job.

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:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

This change expands the critical area for the entire body of the function as opposed to unlocking the vlog stats resource after all the stats have been consumed.

Using the atomic package it could provide an easier to reason about guard against race conditions and reduce the size of the critical area between the semaphore. See: https://golang.org/pkg/sync/atomic/#AddInt64


Reviewed with ❤️ by PullRequest

@jarifibrahim jarifibrahim merged commit 0283a03 into master Aug 9, 2019
@jarifibrahim jarifibrahim deleted the ibrahim/testgetmore branch August 9, 2019 08:01
jarifibrahim pushed a commit that referenced this pull request Mar 12, 2020
The TestGetMore test fails with race condition in updateDiscardStats
function.
See https://teamcity.dgraph.io/viewLog.html?buildId=19032&buildTypeId=Badger_UnitTests

(cherry picked from commit 0283a03)
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.

4 participants