Bug 1020191 - cumin - wrong sorting according to job id
Summary: cumin - wrong sorting according to job id
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: cumin
Version: 2.4
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: 2.5
: ---
Assignee: Trevor McKay
QA Contact: Tomáš Nováčik
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-10-17 09:00 UTC by Tomáš Nováčik
Modified: 2014-04-28 16:47 UTC (History)
8 users (show)

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"
Clone Of:
Environment:
Last Closed: 2014-04-28 16:47:15 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
Overload sort_rows method to JobSummariesAdapter (835 bytes, patch)
2013-12-23 17:26 UTC, Trevor McKay
no flags Details | Diff
Overload sort_rows method to JobSummariesAdapter (956 bytes, patch)
2013-12-23 17:41 UTC, Trevor McKay
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2014:0440 0 normal SHIPPED_LIVE Moderate: Red Hat Enterprise MRG Grid 2.5 security, bug fix, and enhancement update 2014-04-28 20:43:37 UTC

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


Note You need to log in before you can comment on or make changes to this bug.