Tue, 06 Jan 2015 17:07:44 -0800 setdiscovery: drop '_setupsample' usage in '_takequicksample'
Pierre-Yves David <pierre-yves.david@fb.com> [Tue, 06 Jan 2015 17:07:44 -0800] rev 23815
setdiscovery: drop '_setupsample' usage in '_takequicksample' For '_takefullsample' we can just retrieve the list of head directly and ignore the rest of the complex return values. This was the last call to the infamous '_updatesample' function.
Wed, 07 Jan 2015 10:32:17 -0800 setdiscovery: drop the 'always' argument to '_updatesample'
Pierre-Yves David <pierre-yves.david@fb.com> [Wed, 07 Jan 2015 10:32:17 -0800] rev 23814
setdiscovery: drop the 'always' argument to '_updatesample' This argument exists because of the complex code flow in '_takequicksample'. It first gets the list of heads and then calls '_updatesample' on an empty initial sample and a size limit matching the differences between the number of heads and the target sample size. Finally the heads and the sample from '_updatesample' were added. To ensure this addition result had the exact target length, the code had to ensure no elements from the heads were added to the '_updatesample' content and therefore was passing this "always included set of heads". Instead we can just update the initial heads sample directly and use the final target size as target size for the update. This removes the need for this 'always' parameter to the '_updatesample' function The test are affected because different set building order results in different random sampling.
Wed, 07 Jan 2015 17:28:51 -0800 setdiscovery: always add exponential sample to the heads
Pierre-Yves David <pierre-yves.david@fb.com> [Wed, 07 Jan 2015 17:28:51 -0800] rev 23813
setdiscovery: always add exponential sample to the heads As explained in a previous changeset, prioritizing heads too much behaves pathologically when there are more heads than the sample size. To counter this, we always inject exponential samples before reducing to the sample size limit. This already show some benefit in the test themselves, but on a real-world example this moves my discovery for push to pathologically headed repo from 45 rounds to 17 of them. We should maybe ensure that at least 25% of the result sample is heads, but I think the random sampling will be fine in practice.
Wed, 07 Jan 2015 17:23:21 -0800 setdiscovery: directly run '_updatesample'
Pierre-Yves David <pierre-yves.david@fb.com> [Wed, 07 Jan 2015 17:23:21 -0800] rev 23812
setdiscovery: directly run '_updatesample' The heads and exponential sample are going to end up in the same set before any extra processing happens. We simplify the code by directly updating a set with heads. Changes in the order the set is built lead to small changes in the random sampling output. But after double checking, I can confirm the input data to the random sampling is consistent.
Wed, 07 Jan 2015 17:17:56 -0800 setdiscovery: stop using '_setupsample' in '_takefullsample'
Pierre-Yves David <pierre-yves.david@fb.com> [Wed, 07 Jan 2015 17:17:56 -0800] rev 23811
setdiscovery: stop using '_setupsample' in '_takefullsample' Very few of the return values of '_setupsample' remain in use, so we directly retrieve the value we care about and drop the '_setupsample' call.
Wed, 07 Jan 2015 12:09:51 -0800 setdiscovery: randomly pick between heads and sample when taking full sample
Pierre-Yves David <pierre-yves.david@fb.com> [Wed, 07 Jan 2015 12:09:51 -0800] rev 23810
setdiscovery: randomly pick between heads and sample when taking full sample Before this changeset, the discovery protocol was too heads-centric. Heads of the undiscovered set were always sent for discovery and any room remaining in the sample were filled with exponential samples (and random ones if any room remained). This behaved extremely poorly when the number of heads exceeded the sample size, because we keep just asking about the existence of heads, then their direct parent and so on. As a result, the 'O(log(len(repo)))' discovery turns into a 'O(len(repo))' one. As a solution we take a random sample of the heads plus exponential samples. This way we ensure some exponential sampling is achieved, bringing back some logarithmic convergence of the discovery again. This patch only applies this principle in one place. More places will be updated in future patches. One test is impacted because the random sample happen to be different. By chance, it helps a bit in this case.
(0) -10000 -3000 -1000 -300 -100 -30 -10 -6 +6 +10 +30 +100 +300 +1000 +3000 +10000 tip