Summary:
Restores a (simplified and improved) version of Lisk transactions.
This doesn't actually use transactions anywhere yet. DifferentialRevisionEditor
is the #1 (and only?) case where we have transaction problems right now, but
sticking save() inside a transaction unconditionally will leave us holding a
transaction open for like a million years while we run Herald rules, etc. I want
to do some refactoring there separately from this diff before making it
transactional.
NOTE: @jungejason / @nh, can one of you verify these unit tests pass on
HPHP/i/vm when you get a chance? I vaguely recall there was some problem with
(int)$resource. We can land this safely without verifying that, but should check
before we start using it anywhere.
Test Plan: Ran unit tests.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T605
Differential Revision: https://secure.phabricator.com/D1515
Summary:
This is kind of expensive and can be significant on, e.g., the
Maniphest task list view. Do a little more caching and some clever nonsense to
improve performance.
Test Plan:
Local cost on Maniphest "all tasks" view for this method dropped from
##82,856us## to ##24,607us## on 9,061 calls.
I wrote some unit test / microbenchmark things:
public function testGetIDCost() {
$u = new PhabricatorUser();
$n = 100000;
while ($n--) {
$u->getID();
}
$this->assertEqual(1, 1);
}
public function testGetCost() {
$u = new PhabricatorUser();
$n = 100000;
while ($n--) {
$u->getUsername();
}
$this->assertEqual(1, 1);
}
public function testSetCost() {
$u = new PhabricatorUser();
$n = 100000;
while ($n--) {
$u->setID(1);
}
$this->assertEqual(1, 1);
}
Before:
PASS 598ms testSetCost
PASS 584ms testGetCost
PASS 272ms testGetIDCost
After:
PASS 170ms testSetCost
PASS 207ms testGetCost
PASS 29ms testGetIDCost
Also, ran unit tests.
Reviewers: nh, btrahan, jungejason
Reviewed By: nh
CC: aran, epriestley, nh
Differential Revision: https://secure.phabricator.com/D1291
Summary:
I added some type checks in D990 to make sure $columns is an array, but was
overzealous and forgot that loadRawDataWhere needs to be able to take null
as $columns.
Test Plan:
Loaded phabricator and saw the error "Argument 1 passed to LiskDAO::loadRawDataWhere() must be an instance of array, null given" go away
Reviewers: epriestley
CC:
Differential Revision: 991
Summary:
Change the differential typeahead to only load columns that it needs. To do
this, I also enabled partial objects for PhabricatorUser (and made necessary
changes to support this). I also changed the functionality of Lisk's loadColumns
to either accept columns as multiple string arguments or a single array of
strings.
Test Plan:
With tokenizer.ondemand set to false, checked that the typeahead loaded and I
can type multiple people's names. Set tokenizer.ondemand to true and tried
again. In both cases, the typeahead worked.
Reviewers: epriestley
Reviewed By: epriestley
CC: jungejason, aran, epriestley, nh
Differential Revision: 990
Summary:
Added loadColumns, loadColumnsWhere instance methods to Lisk, so when you only
need some fields of your object loaded, you can do so. This will be useful for
places where we fetch a large number of rows, but only care about a few columns.
In that situation, these functions can be used so the db doesn't have to return
as much data.
Test Plan:
Loaded a typeahead to check that the existing lisk functions still work.
Modified typeahead to fetch data using loadColumns instead of loadAll and
checked that it still works.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, nh, jungejason
Differential Revision: 947
Summary:
This diff changes the way Lisk should be used for custom setters and getters,
changing it from having subclasses of Lisk implement their custom setter or
getter to having them override the readField and writeField methods (which get
called by the getters and setters). This diff also has a configurable option
to throw an exception if a subclass of Lisk implements a custom setter or
getter.
Test Plan:
Without the config set to throw, tested in sandbox by browsing differential
and playing with the differential typeahead. With the config set to throw,
tried to load a phabricator page and saw in the error log an exception thrown
by Lisk because of custom getters in PhabricatorUser.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, jungejason, epriestley
Differential Revision: 974
Summary:
Currently you can still punch through Lisk isolation by calling
establishConnection(), and we do that all over the place. Rename getConnection()
to establishConnection() so that all existing callers are safe, and rename
establishConnection() to establishLiveConnection() so that it's not surprising
when this fails to stub in unit tests.
Not wedded to the name if anyone thinks "establishExternalConnection" or
something is clearer.
Test Plan:
Loaded site, browsed around, ran unit tests.
Reviewed By: aran
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran
Differential Revision: 201
Summary:
Allow Lisk to be put into process-isolated mode which establishes
only isolated connections. By default, put it into this mode when running
unit tests. Build some simple unit tests around object insertion and
updating.
NOTE: The one flaw in this is that $dao->establishConnection() still
punches through the isolation layer. I need to do an API change to fix this
though so I'm holding it for now. It will probably just rename getConnection()
to establishConnection() and then rename establishConnection() to something
scary like establishLiveExternalConnection().
Test Plan:
Ran unit tests.
Reviewed By: aran
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran, epriestley
Differential Revision: 194
Summary:
This permits individual deployments to better configure their
database configuration, e.g. to allow more dynamic configuration that reacts
to database moves or master/slave replication.
Test Plan:
Browse
Reviewed By: epriestley
Reviewers: Girish, epriestley
CC: aran, epriestley
Differential Revision: 183
Summary:
Add ability to define mysql slaves and then use that connection on 'r'
connection modes. 'w' connections go to the master server.
Test Plan:
- php -l and checkModule
- worked in my devbox
Reviewed By: jungejason
Reviewers: dpepper, tuomaspelkonen, jungejason
CC: jungejason, aran
Revert Plan:
sure
Differential Revision: 175
