Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Bugfix/issue429 fuzzy #433

Merged
merged 15 commits into from Apr 8, 2015
Merged

Bugfix/issue429 fuzzy #433

merged 15 commits into from Apr 8, 2015

Conversation

pimotte
Copy link
Member

@pimotte pimotte commented Apr 4, 2015

My fuzzy test has been running for a good 20 minutes without yielding any more errors, so I'm going to suggest this branch is finished.

It could be this contains the fix for #429, if that one is caused indirectly by one of these.

@binwiederhier I think these fixes are "obvious" in some sense, but since they concern some critical parts, I want to ask you to look them over.

@@ -236,6 +236,16 @@ private boolean checkPreconditions() throws Exception {
return false;
}

// Check if other operations are running
Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate on why you moved this? It makes sense to check for other operations before LsRemote, but I wonder what made you move it.

@binwiederhier
Copy link
Member

Looks good!

@pimotte
Copy link
Member Author

pimotte commented Apr 5, 2015

I'll add references to this PR where I think it's appropriate. e.g. the SQL queries where really quite wrong in the first place, it was just coincidence it hadn't broken before. (I actually have a strong suspicion now this is the actual cause of #429) Adding a reference to this pull request doesn't really add anything except: "This is how we stumbled upon it." It is in no way the reason for the change.
The ordering with LsRemote is (imo) an appropriate change for a reference, since that race condition is relatively awkward and would likely not be noticed in practice.

pimotte added a commit that referenced this pull request Apr 8, 2015
@pimotte pimotte merged commit 813073f into develop Apr 8, 2015
@pimotte pimotte deleted the bugfix/issue429-fuzzy branch April 8, 2015 07:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants