Bug 1020191

Summary: cumin - wrong sorting according to job id
Product: Red Hat Enterprise MRG Reporter: Tomáš Nováčik <tnovacik>
Component: cuminAssignee: Trevor McKay <tmckay>
Status: CLOSED ERRATA QA Contact: Tomáš Nováčik <tnovacik>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 2.4CC: eerlands, esammons, exo, matt, pbelanyi, sgraf, tmckay, tnovacik
Target Milestone: 2.5   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: cumin-0.1.5796-1 Doc Type: Bug Fix
Doc Text:
Cause: Cumin used string-lexicographic sorting on job ids, instead of sorting by numeric values. Consequence: Incorrect sorts, for example "10" < "2" Fix: Sorting was fixed to order by numeric values. Result: Sorting now as expected, e.g. "2" < "10"
 Story Points: ---
Clone Of: Environment:
Last Closed: 2014-04-28 16:47:15 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Overload sort_rows method to JobSummariesAdapter
none
Overload sort_rows method to JobSummariesAdapter none

Description Tomáš Nováčik 2013-10-17 09:00:45 UTC 
Description of problem:
There is possibility to sort submissions in cumin according to job id. This sorting is done according to string comparison.
Job id title example: localhost.localdomain#100.0, localhost.localdomain#80.0
It consist from domain_name#job_id. Cumin fails to correctly sort submissions which have different count of letters(always when there is difference in number order). 

Version-Release number of selected component (if applicable):
cumin-0.1.5786-2

How reproducible:
100%

Steps to Reproduce:
1.Install cumin according to documentation.
2.Log in as administrator.
3.Go to submission tab and submit 10 jobs with the same description.
3. After jobs completion click on the submission, sort the table according to job id.

Actual results:
Submissions are not correctly sorted. (Not according to the job id)
Ascending sorting result:
localhost.localdomain#1.0
localhost.localdomain#10.0 <-- Error
localhost.localdomain#2.0
localhost.localdomain#3.0
localhost.localdomain#4.0
localhost.localdomain#5.0
localhost.localdomain#6.0
localhost.localdomain#7.0
localhost.localdomain#8.0
localhost.localdomain#9.0

Descending sorting result:
localhost.localdomain#8.0
localhost.localdomain#7.0
localhost.localdomain#6.0
localhost.localdomain#5.0
localhost.localdomain#4.0
localhost.localdomain#3.0
localhost.localdomain#2.0
localhost.localdomain#10.0 <--- Error
localhost.localdomain#1.0

Expected results:
Submissions should be sorted according to actual job id.

Additional info:
Comment 1 Stanislav Graf 2013-11-13 10:42:12 UTC 
Cumin references:
https://fedorahosted.org/grid/wiki/Cumin
https://fedorahosted.org/grid/wiki/CuminAndPersonalCondor
http://tmckayus.github.io/blog/2012/09/24/new-post/

We are interested in:
1] propose patch that fixes the issue
2] propose testing scenario down to individual steps
3] automate testing scenario (from 2]) using your prefered language (optional)

In case you struggle you can:
- ask here in Bugzilla using comment field
- join IRC channels #distcomp and ##cumin on http://freenode.net/
- ask on mailing list (https://fedorahosted.org/grid/wiki/Cumin#OtherUsefulLinks)
Comment 2 Tomas Meszaros 2013-12-11 13:57:28 UTC 
(In reply to Stanislav Graf from comment #1)
> Cumin references:
> https://fedorahosted.org/grid/wiki/Cumin
> https://fedorahosted.org/grid/wiki/CuminAndPersonalCondor
> http://tmckayus.github.io/blog/2012/09/24/new-post/
> 
> We are interested in:
> 1] propose patch that fixes the issue
> 2] propose testing scenario down to individual steps
> 3] automate testing scenario (from 2]) using your prefered language
> (optional)
> 
> In case you struggle you can:
> - ask here in Bugzilla using comment field
> - join IRC channels #distcomp and ##cumin on http://freenode.net/
> - ask on mailing list
> (https://fedorahosted.org/grid/wiki/Cumin#OtherUsefulLinks)

My solution:

1] patch: http://www.fpaste.org/60780/13867700/
2] testing scenario: http://www.fpaste.org/60782/67701031/
3] testing script: http://www.fpaste.org/60784/67701881/
Comment 3 Tomas Meszaros 2013-12-13 21:49:45 UTC 
(In reply to Tomas Meszaros from comment #2)
> (In reply to Stanislav Graf from comment #1)
> > Cumin references:
> > https://fedorahosted.org/grid/wiki/Cumin
> > https://fedorahosted.org/grid/wiki/CuminAndPersonalCondor
> > http://tmckayus.github.io/blog/2012/09/24/new-post/
> > 
> > We are interested in:
> > 1] propose patch that fixes the issue
> > 2] propose testing scenario down to individual steps
> > 3] automate testing scenario (from 2]) using your prefered language
> > (optional)
> > 
> > In case you struggle you can:
> > - ask here in Bugzilla using comment field
> > - join IRC channels #distcomp and ##cumin on http://freenode.net/
> > - ask on mailing list
> > (https://fedorahosted.org/grid/wiki/Cumin#OtherUsefulLinks)
> 
> My solution:
> 
> 1] patch: http://www.fpaste.org/60780/13867700/
> 2] testing scenario: http://www.fpaste.org/60782/67701031/
> 3] testing script: http://www.fpaste.org/60784/67701881/

Improved solution:

1] patch: http://www.fpaste.org/61671/97119913/
2] testing scenario: http://www.fpaste.org/61661/86970126/
3] testing scripts: http://www.fpaste.org/61673/71243138/
                    http://www.fpaste.org/61674/13869712/
Comment 4 Tomas Meszaros 2013-12-20 18:12:12 UTC 
(In reply to Tomas Meszaros from comment #3)
> (In reply to Tomas Meszaros from comment #2)
> > (In reply to Stanislav Graf from comment #1)
> > > Cumin references:
> > > https://fedorahosted.org/grid/wiki/Cumin
> > > https://fedorahosted.org/grid/wiki/CuminAndPersonalCondor
> > > http://tmckayus.github.io/blog/2012/09/24/new-post/
> > > 
> > > We are interested in:
> > > 1] propose patch that fixes the issue
> > > 2] propose testing scenario down to individual steps
> > > 3] automate testing scenario (from 2]) using your prefered language
> > > (optional)
> > > 
> > > In case you struggle you can:
> > > - ask here in Bugzilla using comment field
> > > - join IRC channels #distcomp and ##cumin on http://freenode.net/
> > > - ask on mailing list
> > > (https://fedorahosted.org/grid/wiki/Cumin#OtherUsefulLinks)
> > 
> > My solution:
> > 
> > 1] patch: http://www.fpaste.org/60780/13867700/
> > 2] testing scenario: http://www.fpaste.org/60782/67701031/
> > 3] testing script: http://www.fpaste.org/60784/67701881/
> 
> Improved solution:
> 
> 1] patch: http://www.fpaste.org/61671/97119913/
> 2] testing scenario: http://www.fpaste.org/61661/86970126/
> 3] testing scripts: http://www.fpaste.org/61673/71243138/
>                     http://www.fpaste.org/61674/13869712/

Here is patch_v2: http://www.fpaste.org/63591/87562620/
Comment 7 Trevor McKay 2013-12-23 17:26:00 UTC 
Created attachment 840911 [details]
Overload sort_rows method to JobSummariesAdapter
Comment 8 Trevor McKay 2013-12-23 17:27:47 UTC 
Nice job!

This can be tweaked by moving the sort logic into the JobSummariesAdapter class derived from ObjectQmfAdapter.

The JobSummariesAdapter class is used only for processing the results of a query on a submission to retrieve job summaries, so it is the perfect place.  No need to put guards on the logic to keep from affecting other tables.  Specific knowledge of the record structure at this level is appropriate.

I tested this with a VM, works for me :)

See the attached patch.
Comment 9 Trevor McKay 2013-12-23 17:41:24 UTC 
Created attachment 840913 [details]
Overload sort_rows method to JobSummariesAdapter

Oops, left out the field index test
Comment 11 Tomáš Nováčik 2014-01-28 08:28:38 UTC 
Verified on rhel6x, rhel6i - with cumin-0.1.5796-1.
Comment 13 errata-xmlrpc 2014-04-28 16:47:15 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2014-0440.html