View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000837 | test.cacert.org | test.cacert.org | public | 2010-08-11 00:16 | 2012-06-14 23:18 |
| Reporter | Uli60 | Assigned To | Uli60 | ||
| Priority | normal | Severity | tweak | Reproducibility | sometimes |
| Status | closed | Resolution | suspended | ||
| Platform | Test CAcert Website | OS | N/A | OS Version | Test |
| Summary | 0000837: My Points - Assurances rcvd / given unordered on cacert1.it-sls.de vs production server | ||||
| Description | Assurances received is in an unordered sequence on cacert1.it-sls.de Assurances received seems to be in an ordered sequence on www.cacert.org As the source code of 10.php is copied from the production system tarball and the script does not include an "order by" clause, so it seems that the underlaying msgbase configuration global parameters or multi thread vs. single thread or another behavior results in this difference between testserver and production system. | ||||
| Steps To Reproduce | Login to your account - My Details - My Points script: https://cacert1.it-sls.de/wot.php?id=10 /pages/wot/10.php | ||||
| Additional Information | On the production system it seems there is an automatic ordering by the key index on the testserver system there is no ordering at all | ||||
| Tags | No tags attached. | ||||
| Attached Files | sql_wot_my_points.diff (3,746 bytes)
diff --git a/pages/wot/10.php b/pages/wot/10.php
index 51ed019..4e38f65 100644
--- a/pages/wot/10.php
+++ b/pages/wot/10.php
@@ -23,20 +23,13 @@
</tr>
<tr>
<?
- $query = "SELECT `users`. *, count(*) AS `list` FROM `users`, `notary`
- WHERE `users`.`id` = `notary`.`from` AND `notary`.`from` != `notary`.`to`
- AND `from`='".intval($_SESSION['profile']['id'])."' GROUP BY `notary`.`from`";
+ $query = "SELECT COUNT(1) as `assurances` FROM `notary` WHERE `from`=".intval($_SESSION['profile']['id'])." AND `from` != `to`";
+
$res = mysql_query($query);
$row = mysql_fetch_assoc($res);
- $rc = intval($row['list']);
-/*
- $query = "SELECT `users`. *, count(*) AS `list` FROM `users`, `notary`
- WHERE `users`.`id` = `notary`.`from` AND `notary`.`from` != `notary`.`to`
- GROUP BY `notary`.`from` HAVING count(*) > '$rc' ORDER BY `notary`.`when` DESC";
-*/
- $query = "SELECT count(*) AS `list` FROM `users`
- inner join `notary` on `users`.`id` = `notary`.`from`
- GROUP BY `notary`.`from` HAVING count(*) > '$rc'";
+ $rc = intval($row['assurances']);
+
+ $query = "SELECT COUNT(1) FROM `notary` GROUP BY `from` HAVING COUNT(1) > {$rc}";
$rank = mysql_num_rows(mysql_query($query)) + 1;
?>
@@ -57,16 +50,15 @@
<td class="DataTD"><b><?=_("Method")?></b></td>
</tr>
<?
- $query = "select * from `notary` where `to`='".intval($_SESSION['profile']['id'])."'";
+ $query = "SELECT n.`id`, n.`date`, n.`points`, n.`from` as `from_id`, u.`fname` AS `from_fname`, u.`lname` AS `from_lname`, n.`location`, n.`method` FROM `notary` n LEFT JOIN `users` u ON n.`from`=u.`id` WHERE n.`to`=".intval($_SESSION['profile']['id'])." ORDER BY n.`when` ASC, n.`id` ASC";
$res = mysql_query($query);
while($row = mysql_fetch_assoc($res))
{
- $fromuser = mysql_fetch_assoc(mysql_query("select * from `users` where `id`='".intval($row['from'])."'"));
?>
<tr>
<td class="DataTD"><?=$row['id']?></td>
<td class="DataTD"><?=$row['date']?></td>
- <td class="DataTD"><a href="wot.php?id=9&userid=<?=intval($row['from'])?>"><?=$fromuser['fname']." ".$fromuser['lname']?></td>
+ <td class="DataTD"><a href="wot.php?id=9&userid=<?=intval($row['from_id'])?>"><?=$row['from_fname']." ".$row['from_lname']?></td>
<td class="DataTD"><?=$row['points']?></td>
<td class="DataTD"><?=$row['location']?></td>
<td class="DataTD"><?=_(sprintf("%s", $row['method']))?></td>
@@ -93,17 +85,16 @@
</tr>
<?
$points = 0;
- $query = "select * from `notary` where `from`='".intval($_SESSION['profile']['id'])."' and `to`!='".intval($_SESSION['profile']['id'])."'";
+ $query = "SELECT n.`id`, n.`date`, n.`points`, n.`location`, n.`method`, n.`to` AS `to_id`, u.`fname` AS `to_fname`, u.`lname` AS `to_lname` FROM `notary` n LEFT JOIN `users` u ON n.`to`=u.`id` WHERE n.`from`=".intval($_SESSION['profile']['id'])." AND n.`to`!=".intval($_SESSION['profile']['id'])." ORDER BY n.`when` ASC, n.`id` ASC";
$res = mysql_query($query);
while($row = mysql_fetch_assoc($res))
{
- $fromuser = mysql_fetch_assoc(mysql_query("select * from `users` where `id`='".intval($row['to'])."'"));
$points += $row['points'];
- $name = trim($fromuser['fname']." ".$fromuser['lname']);
+ $name = trim($row['to_fname']." ".$row['to_lname']);
if($name == "")
- $name = _("Deleted before Verification");
+ $name = '<i>'._("Deleted before Verification").'</i>';
else
- $name = "<a href='wot.php?id=9&userid=".intval($row['to'])."'>$name</a>";
+ $name = "<a href='wot.php?id=9&userid=".intval($row['to_id'])."'>$name</a>";
?>
<tr>
<td class="DataTD"><?=intval($row['id'])?></td>
@@ -121,4 +112,3 @@
</tr>
</table>
<p>[ <a href='javascript:history.go(-1)'><?=_("Go Back")?></a> ]</p>
-
| ||||
|
|
This problem we've discussed within the Software/Assessment telco https://wiki.cacert.org/Software/Assessment/20100810-S-A-MiniTOP-telco The problem is twofolded: a) first it shows differences between testserver and production system b) it shows a problem with the production source code (design problem) by using sql queries w/o grouping / ordering the results So the solution path is: 1. analyze the source of the difference problem (i.e. different sql server config global settings?) 2. source code review on scripts that using queries w/o ordering the results a) start with /pages/wot/10.php l.60 current: $query = "select * from `notary` where `to`='".intval($_SESSION['profile']['id'])."'"; patch: $query = "select * from `notary` where `to`='".intval($_SESSION['profile']['id'])."' order by `notary`.`ID`"; l.96 current: $query = "select * from `notary` where `from`='".intval($_SESSION['profile']['id'])."' and `to`!='".intval($_SESSION['profile']['id'])."'"; patch: $query = "select * from `notary` where `from`='".intval($_SESSION['profile']['id'])."' and `to`!='".intval($_SESSION['profile']['id'])."' order by `notary`.`ID`"; |
|
|
If doing changes here, I would propose the attached changes. The SQL is somehow completely rewritten. But there are still improvements possible, especially for the ranking stuff. The additional changes should gain very much performance when loading the My Points page, especially with many assurances. However, I could not test (have no development system) and so this might entierely break by syntax errors, although I tried to run the SQL agains a nearly empty db. |
|
|
Please also note, the current changes change the ordering. If this is not desired, change the DESC in the ordering to ASC (four times). I did not change this from the current development version. |
|
|
Is it conceivable that the result list on the production system happens to be ordered simply because the queried data in the database happens to be ordered? I presume that mysql will not do any reordering when you perform a query without an "order by" statement (but note that I'm not a MySQL expert at all!). I've compared /etc/mysql/my.cnf between webdb and cacert1. There are some small differences, but I don't see how they could affect the issue you are reporting (but again, maybe I dpn't understand enough of MySQL for that). Just to make sure, I have installed the production version of this file now also on cacert1 (including the RCS archive), and restarted mysql. So you might want to re-run your test to check whether anything has changed. --- my.cnf 2010-08-11 21:59:32.000000000 +0200 [original cacert1 version] +++ /etc/mysql/my.cnf 2010-08-11 21:58:11.000000000 +0200 [production versi] @@ -44,7 +44,7 @@ # # Instead of skip-networking the default is now to listen only on # localhost which is more compatible and is not less secure. -#bind-address = 127.0.0.1 +bind-address = 127.0.0.1 # # * Fine Tuning # @@ -70,8 +70,8 @@ # Error logging goes to syslog. This is a Debian improvement :) # # Here you can see queries with especially long duration -#log_slow_queries = /var/log/mysql/mysql-slow.log -#long_query_time = 2 +log_slow_queries = /var/log/mysql/mysql-slow.log +long_query_time = 2 #log-queries-not-using-indexes # # The following can be used as easy to replay backup logs or for replication. I've also looked at: /etc/php4/apache/php.ini /etc/php4/cgi/php.ini /etc/php4/cli/php.ini /home/cacert/etc/php4/apache/php.ini /home/cacert/etc/php4/cgi/php.ini /home/cacert/etc/php4/cli/php.ini For the first three of these there are differences between webdb and cacert1. webdb has some old manually changed versions, while cacert1 has the regular ones coming from a Debian Etch install. But note that this is not really relevant, since those files are *not* referenced by apache/php running in the chroot environment under /home/cacert. The three copies of php.ini under /home/cacert are completely identical between production system and test system, which is not surprising since my script to build the test environment install these as straight copies from the production system. Are there any other global config files that would be relevant for this? I cannot think of any, but maybe you can? |
|
|
As already pointed out in the phone conference, the ordering of query results is unforseeable if not using an ORDER BY statement in the query. So the behaviour is correct. You somehow can do an one time reordering by a key, but this does not changes this behaviour to be deterministic. For this you have to use an ORDER BY clause in the SQL statement. Also have a look at the output when doing a SELECT * on the database. This might show a similar behaviour. But as from what I have learned about databases you should expect a random result order without ORDER BY. However internal things of the database might result in showing a somehow ordered result. However, these internal things should be handled as unforseeable, so I see nothing bad about this. The only thing which should be done if we want to have the result in a certain order is to insert an ORDER BY statement. |
|
|
On 08/12/2010 11:14 AM, Markus Warg wrote: > > explanation is quite simple, IMHO: wot/10.php selects from users, > > notary. This is an SQL join, the order of the result depends on the > > order of the users in the database AND the order of the assurances. The > > order of the users has precedence. Thus if you create a set of users, > > make assurances, you need to create the users AND assurances in the same > > manner/order in two systems to get the same output. I would propose to > > add some decent order by statements (maybe date or users real name or > > ...) to cope with that. > > > > Any comments? Sounds pretty plausible to me ... and moves this issue definitely out of the area of system configuration. When making changes to this particular piece of application code, please take into account the possible performance impact of any; until about a year ago, this command was a performance pig, then some Dirk changes were applied to make the picture a lot brighter. Try not to loose them ... |
|
|
The join is only used for creating the stats at top of the page. However, I dropped it since I think it is unnecessary. The statements in line 60 and 96 which create the list of assurances received/issued do not contain a join. However, they better should. See attached proposals. They have been tested in the meanwhile. More testing needed though (especially for the stats part). This would gain much performance as pointed out earlier. |
|
|
SQL statements w/o 'order by' clause 1. MySQL's default behavior is on a table with a primary key, autoincremented to show the IDs in ascending order on output 2. the order gets changed if some records are deleted and later this unused space will be reused by the DBMS on further inserts 1. can be read in whitepapers and docus of MySQL. Behavior 2. is empirical tested against a MySQL5 database by Dirk and me 2010-08-15 that leads to the assumtion that the reported bug is a default behavior of MySQL as records were deleted before or within the tests. 1. create account 2. add 150 pts administrative increase 3. add autoassurance (-50 pts entry) 4. add 25 assurances (+2 pts each) 5. delete -50 pts assurance entry 6. add 150 administrative increase assurance (w/o check current points, results in 300 pts total) 7. delete last 150 administrative increase 8. add 25 assurances (results in own 0 pts increase) there are also other deletions from other testers that relates to the MySQL behavior seen. Problem identified. |
|
|
Using a descending sort order introduces another side effect. The points issued are shown counting up from the last assurance until a limit of 200 pts is reached. So * is this the desired bahaviour? * should the old behaviour stay? => change to ascending sort order * or remove the limit in the display and show all points? |
|
|
patch 10.php (2010-08-11) has been applied onto the testserver 2010-08-23 (probably by MW) |
|
|
attachment ass-rcvd-sysadmin-console.pdf [^] (112,800 bytes) 2010-08-24 22:24 gives a correct list of assurances received over an account. The points count is related to what has happened in the assurances order. attachment ass-rcvd-my-points.pdf [^] (136,413 bytes) 2010-08-24 22:25 shows the "My Points" view ... especialy the first assurance rcvd listed hasn't happened onto this user !!! so the 100 pts assurance did not happen and/or is not listed in the sysadmins view of points it seems this assurances received under My Points view is totaly wrong |
|
|
The Assurance by me probably just happened between loading these two sites... |
|
|
then it should be read on the next sysadmin view, but it isn't |
|
|
assurances given in sysadmin view lists correct values assurances given in mypoints view lists the wrong column, the assurance points a user gave, but not the effective points (as it happens on the production system) |
|
|
after some analyze and review the patch 10.php [^] (5,246 bytes) 2010-08-11 02:46 is based on dirks ttp-patch ... last points counts :( after rollback order cacert-devel hasn't been rolled back ... |
|
|
10.php [^] (5,246 bytes) 2010-08-11 02:46 removed. |
|
|
sql_wot_my_points.diff also removed |
|
|
new versions, based on cacert.git by mario 10.php [^] (4,802 bytes) 2010-08-25 00:11 sql_wot_my_points.diff [^] (3,746 bytes) 2010-08-25 00:12 |
|
|
Reviewed "My Points" list and seems to be ordered. Nothing bad happned. Sie haben bereits 4 Personen assured und sind damit auf Platz Nummer 15 der Top-Assurer. |
|
|
with 10.php revision not related to https://bugs.cacert.org/view.php?id=827 10.php patch (source from cacert.git, not cacert-devel.git that has been updated with dirk-patches before and isn't in sync with the testserver repository), the points order now all ordered in assurances rcvd and assurances given. no problem regarding removed records (see report 0001642) probably cacert-devel.git needs to be reset to the revision now running on testserver ??!? (null-test), means, no patches at all, identical to the production website + added 10.php patch from bug 0000837 |
|
|
with new 10.php / 15.php this problem seems to be solved. |
|
|
is no longer a problem |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2010-08-11 00:16 | Uli60 | New Issue | |
| 2010-08-11 00:16 | Uli60 | Status | new => needs work |
| 2010-08-11 00:16 | Uli60 | Assigned To | => wytze |
| 2010-08-11 00:32 | Uli60 | Note Added: 0001633 | |
| 2010-08-11 01:27 | Uli60 | Note Edited: 0001633 | |
| 2010-08-11 02:46 | law | File Added: sql_wot_my_points.diff | |
| 2010-08-11 02:46 | law | File Added: 10.php | |
| 2010-08-11 02:49 | law | Note Added: 0001634 | |
| 2010-08-11 03:01 | law | Note Added: 0001637 | |
| 2010-08-11 20:41 | wytze | Note Added: 0001638 | |
| 2010-08-11 23:48 | law | Note Added: 0001639 | |
| 2010-08-12 09:42 | wytze | Note Added: 0001640 | |
| 2010-08-12 14:45 | law | Note Added: 0001641 | |
| 2010-08-16 11:32 | Uli60 | Note Added: 0001642 | |
| 2010-08-16 11:36 | Uli60 | Assigned To | wytze => Uli60 |
| 2010-08-24 22:07 | law | Note Added: 0001671 | |
| 2010-08-24 22:22 | Uli60 | Note Added: 0001672 | |
| 2010-08-24 22:24 | Uli60 | File Added: ass-rcvd-sysadmin-console.pdf | |
| 2010-08-24 22:25 | Uli60 | File Added: ass-rcvd-my-points.pdf | |
| 2010-08-24 22:29 | Uli60 | Note Added: 0001673 | |
| 2010-08-24 22:41 | law | Note Added: 0001674 | |
| 2010-08-24 22:42 | Uli60 | Note Added: 0001675 | |
| 2010-08-24 22:43 | Uli60 | Note Added: 0001676 | |
| 2010-08-24 23:50 | Uli60 | Note Added: 0001677 | |
| 2010-08-25 00:10 | Uli60 | File Deleted: 10.php | |
| 2010-08-25 00:10 | Uli60 | Note Added: 0001678 | |
| 2010-08-25 00:11 | Uli60 | File Deleted: sql_wot_my_points.diff | |
| 2010-08-25 00:11 | law | File Added: 10.php | |
| 2010-08-25 00:11 | Uli60 | Note Added: 0001679 | |
| 2010-08-25 00:12 | law | File Added: sql_wot_my_points.diff | |
| 2010-08-25 00:12 | Uli60 | Note Added: 0001680 | |
| 2010-09-01 02:05 | law | Note Added: 0001696 | |
| 2010-09-01 13:12 | Uli60 | Note Added: 0001699 | |
| 2011-06-14 11:22 | NEOatNHNG | Category | => cacert1.it-sls.de |
| 2011-09-15 15:27 | Uli60 | Note Added: 0002442 | |
| 2011-09-15 15:27 | Uli60 | Status | needs work => solved? |
| 2011-09-15 15:27 | Uli60 | Resolution | open => suspended |
| 2011-09-15 15:28 | Uli60 | Status | solved? => needs feedback |
| 2011-09-15 15:28 | Uli60 | Resolution | suspended => reopened |
| 2011-09-15 15:29 | Uli60 | Relationship added | related to 0000827 |
| 2011-09-15 15:29 | Uli60 | Status | needs feedback => solved? |
| 2011-09-15 15:29 | Uli60 | Resolution | reopened => suspended |
| 2011-10-26 00:56 | Uli60 | Note Added: 0002657 | |
| 2011-10-26 00:56 | Uli60 | Status | solved? => closed |
| 2012-06-14 23:18 | Uli60 | Relationship added | related to 0001074 |
| 2018-06-19 18:12 | egal | Category | cacert1.it-sls.de => test.cacert.org |
