View Issue Details

IDProjectCategoryView StatusLast Update
0000837test.cacert.orgtest.cacert.orgpublic2012-06-14 23:18
ReporterUli60 Assigned ToUli60  
PrioritynormalSeveritytweakReproducibilitysometimes
Status closedResolutionsuspended 
PlatformTest CAcert WebsiteOSN/AOS VersionTest
Summary0000837: My Points - Assurances rcvd / given unordered on cacert1.it-sls.de vs production server
DescriptionAssurances 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 ReproduceLogin to your account - My Details - My Points
script: https://cacert1.it-sls.de/wot.php?id=10
/pages/wot/10.php

Additional InformationOn the production system it seems there is an automatic ordering by the key index
on the testserver system there is no ordering at all
TagsNo tags attached.

Relationships

related to 0000827 closedegal Main CAcert Website Tverify points to be deprecated 
related to 0001074 closedUli60 Main CAcert Website Wrong display of method on points page wot.php?id=10 

Activities

Uli60

2010-08-11 00:32

updater   ~0001633

Last edited: 2010-08-11 01:27

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`";

law

2010-08-11 02:49

administrator   ~0001634

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.

law

2010-08-11 03:01

administrator   ~0001637

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.

wytze

2010-08-11 20:41

developer   ~0001638

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?

law

2010-08-11 23:48

administrator   ~0001639

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.

wytze

2010-08-12 09:42

developer   ~0001640

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 ...

law

2010-08-12 14:45

administrator   ~0001641

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.

Uli60

2010-08-16 11:32

updater   ~0001642

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.

law

2010-08-24 22:07

administrator   ~0001671

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?

Uli60

2010-08-24 22:22

updater   ~0001672

patch 10.php (2010-08-11) has been applied onto the testserver
2010-08-23 (probably by MW)

2010-08-24 22:24

 

2010-08-24 22:25

 

ass-rcvd-my-points.pdf (136,413 bytes)

Uli60

2010-08-24 22:29

updater   ~0001673

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

law

2010-08-24 22:41

administrator   ~0001674

The Assurance by me probably just happened between loading these two sites...

Uli60

2010-08-24 22:42

updater   ~0001675

then it should be read on the next sysadmin view, but it isn't

Uli60

2010-08-24 22:43

updater   ~0001676

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)

Uli60

2010-08-24 23:50

updater   ~0001677

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 ...

Uli60

2010-08-25 00:10

updater   ~0001678

10.php [^] (5,246 bytes) 2010-08-11 02:46
removed.

2010-08-25 00:11

 

10.php (4,802 bytes)

Uli60

2010-08-25 00:11

updater   ~0001679

sql_wot_my_points.diff also removed

2010-08-25 00:12

 

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&amp;userid=<?=intval($row['from'])?>"><?=$fromuser['fname']." ".$fromuser['lname']?></td>
+    <td class="DataTD"><a href="wot.php?id=9&amp;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&amp;userid=".intval($row['to'])."'>$name</a>";
+			$name = "<a href='wot.php?id=9&amp;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>
-
sql_wot_my_points.diff (3,746 bytes)   

Uli60

2010-08-25 00:12

updater   ~0001680

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

law

2010-09-01 02:05

administrator   ~0001696

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.

Uli60

2010-09-01 13:12

updater   ~0001699

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

Uli60

2011-09-15 15:27

updater   ~0002442

with new 10.php / 15.php
this problem seems to be solved.

Uli60

2011-10-26 00:56

updater   ~0002657

is no longer a problem

Issue History

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