Skip to content

Conversation

@kofalt
Copy link
Contributor

@kofalt kofalt commented Jan 18, 2018

Before:

api/jobs/stats

{
    "by-state": {
        "cancelled": 0, 
        "complete": 0, 
        "failed": 0, 
        "pending": 74, 
        "running": 0
    }, 
    "by-tag": [
        {
            "count": 37, 
            "tags": [
                "auto", 
                "dicom-mr-classifier"
            ]
        }, 
        {
            "count": 37, 
            "tags": [
                "auto", 
                "dcm2niix"
            ]
        }
    ], 
    "permafailed": 0
}

After:

api/jobs/stats

{
    "states": {
        "cancelled": 0, 
        "complete": 0, 
        "failed": 0, 
        "pending": 74, 
        "running": 0
    }
}
api/jobs/stats?all=1

{
    "recent": {
        "cancelled": [], 
        "complete": [], 
        "failed": [], 
        "pending": [
            {
                "_id": "5a60d7a9fdc9bc001d4b90fe", 
                "modified": "2018-01-18T17:21:45.923000+00:00"
            }, 
            {
                "_id": "5a60d7a9fdc9bc001d4b90fd", 
                "modified": "2018-01-18T17:21:45.918000+00:00"
            }, 
            {
                "_id": "5a60d7a9fdc9bc00184b9113", 
                "modified": "2018-01-18T17:21:45.448000+00:00"
            }
        ], 
        "running": []
    }, 
    "states": {
        "cancelled": 0, 
        "complete": 0, 
        "failed": 0, 
        "pending": 74, 
        "running": 0
    }, 
    "unique": [
        "auto", 
        "dcm2niix", 
        "dicom-mr-classifier"
    ]
}
api/jobs/stats?last=1&tags=dcm2niix

{
    "recent": {
        "cancelled": [], 
        "complete": [], 
        "failed": [], 
        "pending": [
            {
                "_id": "5a60d7a9fdc9bc001d4b90fd", 
                "modified": "2018-01-18T17:21:45.918000+00:00"
            }
        ], 
        "running": []
    }, 
    "states": {
        "cancelled": 0, 
        "complete": 0, 
        "failed": 0, 
        "pending": 37, 
        "running": 0
    }
}
api/jobs/stats?unique=1

{
    "states": {
        "cancelled": 0, 
        "complete": 0, 
        "failed": 0, 
        "pending": 74, 
        "running": 0
    }, 
    "unique": [
        "auto", 
        "dcm2niix", 
        "dicom-mr-classifier"
    ]
}

Before:

c := api.NewApiKeyClient("localhost:8443:s8zx9pVmOOR7COADuJ", api.InsecureNoSSLVerification)

_, errChan := c.DownloadSimple(
	"jobs/123/config.json", 
	&api.DownloadSource{Writer: NopWriteCloser(new(bytes.Buffer))}
)

Println(<-errChan)
Response body was expected to be of length -1 but was instead 1138

After:

<nil>

@codecov-io
Copy link

codecov-io commented Jan 18, 2018

Codecov Report

Merging #1054 into master will increase coverage by 0.02%.
The diff coverage is 97.5%.

@@            Coverage Diff             @@
##           master    #1054      +/-   ##
==========================================
+ Coverage   91.15%   91.18%   +0.02%     
==========================================
  Files          50       50              
  Lines        6896     6928      +32     
==========================================
+ Hits         6286     6317      +31     
- Misses        610      611       +1

@kofalt kofalt force-pushed the job-improvements branch 6 times, most recently from eda28f5 to 324a36e Compare January 19, 2018 15:57

# List recently modified jobs for each state
if last is not None:
results['recent'] = {s: config.db.jobs.find({'$and': [match, {'state': s}]}, {'modified':1}).sort([('modified', pymongo.DESCENDING)]).limit(last) for s in JOB_STATES}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than doing N (job state) finds we could use a mongo aggregation pipeline that does a sort, followed by a group (by state), followed by a limit. I'm not convinced that would be faster on larger datasets but it's something to think about if this ends up being a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually looked into that, and it was slower - via the entirely scientific process of running each query a few times and checking the execution time. It was close though, and dev.fw may not be very representative for this either... It would at least be fewer round trips!

@nagem
Copy link
Contributor

nagem commented Jan 22, 2018

LGTM, I left an option for another route we could take if the recent jobs listing by state becomes a problem.

@kofalt kofalt merged commit 1678dff into master Jan 22, 2018
@kofalt kofalt deleted the job-improvements branch January 22, 2018 17:35
ryansanford pushed a commit that referenced this pull request Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants