Test Plan: this is fixing some tests
Reviewers: #sourcecontrol, quark, ttung
Reviewed By: quark
Differential Revision: https://phabricator.fb.com/D3117092
Signature: t1:3117092:1459371915:231c133209a06f3e0e207482858775802d5820e7
Summary:
We've been seeing bugs where pulls result in bookmarks missing for clients. We
suspect it to be a race condition on the server side. One possible race is that
we load the changelog data before the bookmarks, and therefore may receive
bookmarks that point to a node that isn't in memory.
A simple fix to decrease the chances of this is to force loading bookmarks
first. Since hgsql is the first bit of code that access the repo, we can just
change the order here to ensure bookmarks are loaded first.
Test Plan:
It's not really a correctness change, so I wasn't able to test it.
What I did do however was insert exceptions into the bookmark loading and
changelog loading logic on one of our servers. This showed me that the changelog
was indeed being loaded first before because of this code.
Reviewers: #sourcecontrol, ttung
Reviewed By: ttung
Differential Revision: https://phabricator.fb.com/D3063659
Signature: t1:3063659:1458244130:7448377065fa0f0f6d98e43eaa00bdfc380948b6
Summary:
We are overriding the _afterlock to mind one more lock before executing
callbacks. We need that so our post commit hooks are executed out of the lock.
Test Plan: run tests
Reviewers: #sourcecontrol, ttung, durham
Reviewed By: durham
Differential Revision: https://phabricator.fb.com/D2809038
Signature: t1:2809038:1452124328:3b1d2da7711cdf1c97fe06d5dcf07be253582fcd
Summary:
Upstream has moved almost all bookmark writes to use bookmark.recordchange()
instead of write, which works inside a transaction. For hgsql, we need to
perform our bookmark writes inside a transaction now.
Additionally, we need to check self.disablesync inside committodb. disablesync
is used to allow us to write bookmarks during a syncdb without triggering the
bookmark writes to the db (since that would be a infinite loop). Previously we
only needed the syncdb check for bookmark writes, but since that has moved to
transactions, we need to do the check there too (committodb fires on
pretxnclose).
Lastly, in theory we could remove our wrapping of bookmark.write now, since
nothing should use it. But since upstream still has code for bookmark.write to
write bookmarks when outside a transaction, we can't remove our code just yet,
in case some extension still uses bookmark.write.
Test Plan: Ran the tests
Reviewers: ttung, lcharignon, #sourcecontrol, mitrandir
Reviewed By: mitrandir
Differential Revision: https://phabricator.fb.com/D2808601
Signature: t1:2808601:1452116992:ed34107a97ac633281ad8934266b35e3375cb247
Summary:
We've seen load issues on the mysql server in certain situations. This is caused
by large revision_reference query load from every read connection checking the
database.
To fix it, let's use Mercurial locks to enforce that only one reader is checking
and syncing at any given time. All other will just serve the existing on disk
data immediately.
Writer clients are unaffected. They do not obey the rate limiting lock and do
exactly what they did before.
Test Plan: I'll try to add a test
Reviewers: #sourcecontrol, ttung
Differential Revision: https://phabricator.fb.com/D2756217
Summary:
hgsql failed when run against bundle repos before. We've fixed it so that no
sync is performed when reading from a bundle repo, but writes will be rejected.
Test Plan: Added a test and ran the tests
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2665347
Summary:
The old MySQLdb library did not support IPv6 so we've switched to using
mysql.connector, which is more recent and does support IPv6.
The most notable change in functionality is that it returns byte arrays instead
of strings, so I had to add a custom type converter that returns strings.
Test Plan: Ran the tests (after installing the new rpm)
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2665344
Summary:
We're going to be refactoring which functions actually sync with sql soon, and
putting this logic in a function will help us stay clean.
Test Plan: Ran the tests
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2665341
Summary:
In order to increase commit throughput on the server side, we need to be able to
run hooks and do caching outside of the lock. Upstream Mercurial and the
pushrebase extension are doing this by making the unbundle lock possible to
acquire lazily.
For hgsql, this means we can't just wrap the entire unbundle command in a lock.
Instead we need to take the lock only when it's requested.
Test Plan:
Wrote a test, and changed the existing bundle2 test to use the lazy
locking
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2525414
Summary:
In a future patch we need to be able to manipulate the sqlsync/lock logic more
freely than the current executewithsql allows. This patch refactors the sql
sync/lock logic into a context class and changes executewithsql to use it.
Test Plan: Ran the tests
Reviewers: pyd, sid0, #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2521341
Summary:
Previously, any time we needed to edit the heads or bookmarks in the
revision_references table, we would delete all entries for that reponame and
insert them again. This ends up being very expensive and creates a large series
of deletes and inserts that need to be replicated to slave dbs.
This patch does the minimal number of deletions and inserts to update the table.
Test Plan: Ran the tests
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2534693
Summary:
The manifest cache is important for push performance, so let's keep it even
after we sync with the db. Previously repo.manifest was just invalidated and
thrown away.
This is safe because the manifest cache is key'd on the node, which will never
result in a stale cache.
Test Plan:
Ran the tests. And manually applied it to a mercurial server with a
bunch of printf's to make sure the exact cache instance survived the sync.
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2491693
Summary:
Since all the unbundle logic has been moved down into exchange.unbundle, we only
need to wrap that. In addition, not wrapping wireproto.unbundle allows us to use
that function in the pushrebase extension to prefetch the manifest contents
outside of the sql lock.
Test Plan:
Ran the tests. Also manually applied it on one server and verified
that pushrebase's prefetch occurred outside of the sql lock.
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2491688
e0a0f9ad3e4c in upstream Mercurial caused the filecache entries to have their
stat times updated when a transaction closed. This meant that our changelog
wasn't actually up to date, but the cache thought it was. And it turns out
self.invalidate() doesn't actually invalidate those time stamps.
The fix is to manually destroy the file caches that we know we've invalidated.
Summary:
The lock held timer was also including how long the lock had been waited on,
which was wrong. Let's break it into two timers.
Also fix an annoying indentation error.
Test Plan: Ran the tests
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2474908
Summary:
We were using the revlog data length to know if we needed to commit to the
database. This is wrong since empty revlogs are allowed. This would only
trigger when a commit was made that *only* merged files and didn't change any
history at all.
This bug was introduced by the recent batching improvements.
Test Plan:
Ran the tests. We should probably write a test covering this, but we
need to get this deployed asap.
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2439674
Summary:
Previously we only used locktimeout for configuring the connection's innodb lock
timeout. Let's also use it for the actual GET_LOCK() call.
When combined with the pushrebase extension, there can be situations where
someone else is doing a large push, and we don't want to fail the local client's
push just because it's taking a while. So this config gives us the ability to
configure how long the push will wait before giving up.
Test Plan: Ran the tests
Reviewers: #sourcecontrol, ericsumner
Reviewed By: #sourcecontrol, ericsumner
Differential Revision: https://phabricator.fb.com/D2419968
Summary:
We've been seeing the occasional spurious connection issues. Let's do some basic
retrying when connecting to mysql.
Test Plan: It's probably fine!
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2419942
Summary:
Previously we did a mysql insert for every revision that was being inserted.
This involves a lot of round trips and slows the push down dramatically. Let's
batch up inserts instead.
Test Plan:
Ran the tests and already deployed it to a hg server and pushed to
it.
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2396633
Summary:
I was seeing errors where mysql timed about because it was fetching too much
data during the rev validation stage. So let's change it to verify the revs in
batches.
Test Plan:
Tests pass, I also deployed it to a hg server and pushed changes to
it.
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2396632
OperationalError exceptions were being hidden by "MySQL server has gone away"
exceptions caused by the unlock. We already accounted for certain kinds of
exceptions, so we just need to add the other type to the list.
Tested it by hotfixing a server.
Summary:
We've seen some hg processes hang on the Mercurial servers because
mysql throws an exception on the background thread and the foreground thread
just waits forever.
This patch catches background exceptions and sends them to the foreground for
throwing.
Test Plan:
I'm not sure how to cause an exception via test, but I manually
verified it works by adding a 'raise Exception(...)' to the background thread
and running the tests and verifying the exception showed up.
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2337535
Summary:
We suspect that these delete statements sometimes cause full table
scans which lock up the table and cause lock wait timeout errors from other
repositories. By forcing the index, we should prevent the scan and prevent
these errors from happening, increasing hgsql throughput.
Test Plan: ./run-tests.py
Reviewers: #sourcecontrol, durham, ebergen
Reviewed By: ebergen
Subscribers: durham
Differential Revision: https://phabricator.fb.com/D2235417
Tasks: 7462484
Signature: t1:2235417:1436579460:b0d0193db4c2d799c8263fb2225b70b8762eb06f
Summary:
pretxnclose hooks were incorrectly running during the sql sync. Since the
commits have already been committed (on another machine), there's no need to run
the hooks, so let's disable them.
Test Plan: Added a test
Reviewers: pyd, lcharignon, akushner, rmcelroy
Differential Revision: https://phabricator.fb.com/D2173886
Summary:
The hg-ssh wrapper prevents all transactions from running, including the one we
use to sync our repositories from hgsql. This patch updates hgsql to override
that hook temporarily during syncs.
This wasn't an issue before because upstream mercurial used to hook on
prechangegroup which hgsql did not fire.
Test Plan: Added a test. Verified it failed before, and passes now.
Reviewers: pyd, lcharignon, rmcelroy, sid0
Differential Revision: https://phabricator.fb.com/D2148580
Summary:
This can't hurt anything, and it makes sure we have a fresh
transaction for the references which have been timing out, strangely.
Test Plan: run_tests.py
Reviewers: durham
Reviewed By: durham
Subscribers: durham
Differential Revision: https://phabricator.fb.com/D2137954
Signature: t1:2137954:1433811692:1234cde23efd2fa8a95ad87453390aa8edeb335d
Summary: this may be more performant in practice
Test Plan: run tests
Reviewers: durham
Reviewed By: durham
Subscribers: durham
Differential Revision: https://phabricator.fb.com/D2137916
Signature: t1:2137916:1433811302:32c45a163caa70654150945e3dc62ca197331b65
We're encountering server failures with "Lock wait timeout exceeded" issues.
Let's allow us to crank up the timeout via a config.
Also update a test which is mildly out of date.
Summary:
Upstream has changed bundle2 and the transaction hooks. We need to update to
match them. There is now a validator function on transactions that runs the
pretxnclose hooks. We now wrap that to make sure our db commit happens after
the hooks have run.
Test Plan: Fixed the tests to cover the new hooks
Reviewers: sid0, rmcelroy, lcharignon, pyd, ericsumner
Reviewed By: ericsumner
Differential Revision: https://phabricator.fb.com/D2015133
Signature: t1:2015133:1429904252:d0f1fe226df1cc97a3e1da0d9dd6b2ac6ccf8a1a
Upstream Mercurial now takes the source repo lock during a push because it might
update the source phase data. Consequently, we need to wrap this with a sqllock.
A test caught this.
Summary:
Bookmarks could get prematurely written to sql if hooks were invoked
during a bundle2 transaction. If the transaction was then aborted, we could be
left in a state where the bookmarks had been written, but the commits had not.
The fix is to only write bookmarks outside the transaction if bookmarks.write()
is explicitly called. Otherwise we write them as part of transactionclose, just
like the revlog entries.
Test Plan: Ran the tests.
Reviewers: sid0, pyd
Reviewed By: pyd
Differential Revision: https://phabricator.fb.com/D1711961
Signature: t1:1711961:1417549773:044611a63de2376c64c218404cdb1e04337ced3d
Summary:
I made a last minute change to the last diff that broke it. Thought it through
and got the intended fix behavior while still being correct.
Test Plan: actually run the test
Reviewers: durham, sid0, davidsp
Differential Revision: https://phabricator.fb.com/D1683661
Summary:
Previously, the --forcesync flag would cause the command to acquire the
sqllock, which would mean a read only command would block for any ongoing write
commands to complete, which is undesirable. Now, we will wait for the lcoal repo
lock only, instead of skipping the sync as we did before the initial patch.
This ensures that forcesync operations will see all committed changes, but won't
wait for ongoing write transactions from other masters.
Test Plan: updated test
Reviewers: sid0, davidsp, pyd, mpm, durham
Reviewed By: durham
Differential Revision: https://phabricator.fb.com/D1682970
Signature: t1:1682970:1416014352:e7f3daa09e79abd43a16c8eb74bc69272d73c83d
Local pushing and pulling with bundle two goes through exchange.unbundle, so we
need to wrap that now. Also did some minor grouping of the wrapping's. The only
new one is for exchange.unbundle.
Tested it by enabling bundle2 for all the tests and running it without my change
and with my change.
Summary:
When enabled, forcesync enables a read-only caller to force a DB sync. This
allows a simple command like 'hg log -l 1' to still ensure a full sync occurs.
Test Plan: updated test
Reviewers: durham, sid0, davidsp, pyd, mpm
Reviewed By: durham
Differential Revision: https://phabricator.fb.com/D1643055
Tasks: 5456657
Upstream Mercurial no longer always calls bookmark.write to serialize the
bookmarks. Now it sometimes goes through the transaction API which calls
bookmark._write.
Summary:
The sqlstrip command had a bug where a multidigit revision, like '5381' would be
treated like a string, sorted, and the first digit taken. So we always
sqlstripped one of the first revs in the repository.
This fixes that to treat rev like an integer, and removes the sort (which was a
legacy bit of code from when sqlstrip supported multiple integers).
Test Plan: Ran the tests.
Reviewers: sid0, pyd, davidsp
Differential Revision: https://phabricator.fb.com/D1580556
