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.
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.
Check with the actual database to ensure we are the ones with the write lock
before performing the actual lock. We encountered an issue previously where two
writers managed to write at once, which this should now catch.
Summary:
In certain pull use cases (hg pull -B foo), it tries to update the bookmarks
outside of the normal bookmark.updatefromremote() function. This takes the lock
at the very beginning of the pull so all inner bookmark manipulations should
succeed.
Test Plan:
Ran the new test with and without the fix, verified it failed before
and passed afterwards.
Reviewers: sid0, davidsp
Differential Revision: https://phabricator.fb.com/D1214148
Pushing and pulling local bookmarks was a bit broken, since as soon as the
bookmark dict tried to write, it would sync with the db, which overwrote the
current bookmarks.
This changes it to require the sql connection to be set up and locked before
the bookmark modifications even start to happen.
Previously, if the repo needed to be synced, every command would block and
wait until the repo was synced. So if one command was doing the update, all
the other parallel commands (even the readonly ones) would be blocked.
This changes it so readonly commands (i.e. commands that don't require locking
the db) can be executed even if the repo is slightly out of date. The downside
of this is a user may get a slightly old (by seconds) version of the repo when
doing a pull.
Bookmarks were failing to write to the database when pushed (but worked fine
when pulled) because the executewithsql around the bookmark writing would sync
the bookmark dict and overwrite the pending bookmarks.
The new logic takes the lock up at the pushkey level. Also had to add support
for nesting locks so both the pushkeys and the bookmark write could take the
bookmarks_lock.
MySQL has limits on how large a transaction can become, so if a commit is
larger than that we need to break it into several database transactions. To
keep the atomic nature of the repo table, we add a new pushkey namespace 'tip'
that contains the latest valid linkrev for that repo in the table. Then at the
very end we update the 'tip' marker (along with the heads).
When syncing, the repo only syncs up to the tip commit.
When making a commit, it first cleans up any abandon commits by deleting
any rows that have a linkrev greater than the tip.
Before writing revisions to the database we now validate that they are
correct. This consists of two checks:
1. Are we appending to the same linkrev in the database as we are in the local
repo.
2. Every rev that we are writing to the database has a base, p1, and p2 rev
dependency. Before we write the rev, verify that the base, p1, and p2 revs in
the database have the same nodes as in the localrepo. This prevents us from
writing rev dependencies that then point at the incorrect node (due to
different ordering in the db vs local).
In order to validate that the incoming revisions are valid, we need node
information in the revisions table. A future patch will add the actual
validation.
Check that heads and bookmarks match after syncing.
Verify that the new commits are being applied on top of the correct linkrev on
the server.
Remove excess invalidate steps. They are no longer necessary since we do the
lock.release() much later in the process (which was what caused problems
originally).
I added revlog._writeentry to upstream Mercurial which allows us to easily
intercept revlog writes instead of building crazy file-like objects to
intercept the calls.
