I accidentally checked in a hardcoded path. Let's drop that. Also hide stderr
output from the mysql executions. This can sometimes complain about the password
being in plaintext.
Summary:
Adds a test specifically covering the interaction between pushrebase
and hgsql, to ensure that the sql lock is not held while pushrebase is executing
the prepushrebase hook.
Test Plan: ran it
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.fb.com/D2527791
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
Summary: Mercurial rev f8c397b4362c changed the API for `phases.advanceboundary`. A different rev also added a benign 'activating bookmark' message.
Test Plan: Ran the tests.
Reviewers: durham
Reviewed By: durham
Differential Revision: https://phabricator.fb.com/D1561665
Tasks: 5170539
The previous change that allowed transactions that didn't touch revlogs was
broken. It allowed transactions like 'strip', etc. This reverts that.
To fix the actual issue we were trying to fix before, we now add a sql
transaction around local phase moves as part of exchange.
Recent upstream Mercurial changes have moved the phase cache into a transaction.
We don't sync phases as part of hgsql (everything is just public), so lets allow
transactions as long as they don't touch revlogs.
This was necessary because pushing between local repos was broken by this.
Summary:
Adds an 'hg sqlstrip' command that strips commits from the local repo and from
the database. Since this modifes the database, all other server hosts will
suddenly become out of sync, so ideally this command should be run on all the
server hosts at the same time.
Test Plan: Added a test and ran it
Reviewers: sid0, pyd, dschleimer, davidsp
Reviewed By: davidsp
Differential Revision: https://phabricator.fb.com/D1422865
Commit 70d94df78f11 tried to hide this benign warning, but did it at the wrong
spot. This does it at the right spot (around cursor.close()).
This is the description from the original commit:
The revision_references table has multiple unique keys (primary key, and a unique
name index). Using ON DUPLICATE KEY in an INSERT causes a warning, since it will
only update one of the rows, even if there are two duplicates. We can safely
ignore this since we know there will only ever be one row that has a duplicate
key.
Previously we escaped path names ourselves, but this didn't remove %'s from the
paths, which could break the sql libraries ability to format queries with args.
Now we use the actual sql libraries builtin formatting.
