Page MenuHomeIn-Portal Phabricator

INP-1762 - Destroy query object before logging
ClosedPublic

Authored by alex on Apr 19 2019, 10:38 AM.

Details

Test Plan

Preparations

  • in IDE:
    1. enable debug mode
  • in Web Browser
    1. open Front-End
    2. remember SQL count in Debugger (in my case it was 21 sql (1 of SQLs was repeated twice + 1 sql was non-loggable because it wasn't a SELECT = 19 loggable sqls)
  • in IDE:
    1. disable debug mode
    2. open /system/debug.php file for editing
    3. uncomment DBG_CAPTURE_STATISTICS and DBG_MAX_SQL_TIME constant declarations
    4. set 0 (instead of current value of 2) as value for DBG_MAX_SQL_TIME constant
    5. save changes
    6. open /index.php file for editing
    7. after $application->Init(); line add this code:
$rows = $application->Conn->GetIterator('SELECT * FROM ' . TABLE_PREFIX . 'Languages', null, true);
echo '[is_iterator:' . (is_object($rows) ? 'yes' : 'no') . ']';
  1. save changes

Plan (part 1 - general & iterator logging)

  • in Database Manager:
    1. clear contents of SlowSqlCapture and StatisticsCapture tables in the database
  • in Web Browser
    1. go to Front-End
    2. confirm, that no Fatal Error happened
    3. confirm, that [is_iterator:yes] text is displayed somewhere on the page (before the patch it would have no instead of yes
  • in Database browser
    1. open SlowSqlCapture table
    2. confirm, that record for each unique SQL (was 19 records in my case + one of records has Hits=2) are created
    3. confirm, that records mentioning SlowSqlCapture and StatisticsCapture tables are not logged

Plan (part 2 - load balancer logging)

  • in Database Manager:
    1. clear contents of SlowSqlCapture and StatisticsCapture tables in the database
  • in IDE:
    1. open /system/config.php file for editing
    2. add this code fragment at the end (adjust DB username/password to match the ones you're using):
$_CONFIG['Databases'] = array(
	array(
		'DBHost' => 'localhost',
		'DBUser' => 'db-username',
		'DBUserPassword' => 'db-password',
		'DBLoad' => 1,
	),
);
    1. save changes
  • repeat tests from Plan (part 1 - general & iterator logging)
  • one extra SQL (the SHOW PROCESSLIST would be logged, when load balancer is used, but that's fine)

Diff Detail

Repository
rINP In-Portal
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

alex created this revision.Apr 19 2019, 10:38 AM
alex requested review of this revision.Apr 19 2019, 10:38 AM
alex added a project: Restricted Project.Apr 19 2019, 10:42 AM
alex updated this revision to Diff 897.Apr 25 2019, 2:41 AM
  1. make sure, that SELECT queries used for slow query logging aren't counted towards total query count limit
  2. [bugfix] attemt to get non-debug database query iterator while in debug mode would result in an exception
alex edited the test plan for this revision. (Show Details)Apr 25 2019, 6:16 AM
alex updated this revision to Diff 898.Apr 25 2019, 6:21 AM

Introduce global flag, that allows to disable logging for slow queries for a block of sqls.

alex updated this revision to Diff 899.Apr 25 2019, 6:22 AM

Fixed a typo.

erik added a comment.Apr 25 2019, 7:25 AM

Part 1 - general logging - tested. No issues found. Waiting for part 2 and part 3 of the test plan.

alex planned changes to this revision.Apr 25 2019, 7:25 AM

Changed the code. Need to update test plan.

alex edited the test plan for this revision. (Show Details)Apr 25 2019, 7:50 AM
alex edited the test plan for this revision. (Show Details)
alex edited the test plan for this revision. (Show Details)Apr 25 2019, 7:54 AM
alex updated this revision to Diff 900.Apr 25 2019, 8:16 AM

Sending latest changes (maybe nothing was changed), but just in case.

alex added a comment.Apr 25 2019, 8:18 AM

Please retest all from scratch, because I've rewritten test plan.

erik accepted this revision.Apr 25 2019, 9:15 AM
This revision is now accepted and ready to land.Apr 25 2019, 9:15 AM
This revision was landed with ongoing or failed builds.Jan 5 2021, 3:06 AM
Closed by commit rINP16662: Fixes INP-1762 - Destroy query object before logging (authored by alex, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.