Add performance reports #107

jrbourbeau · 2022-04-29T15:10:33Z

This uploads performance reports for tests

ncclementi · 2022-05-01T20:50:07Z

conftest.py

    with Client(small_cluster) as client:
        small_cluster.scale(10)
        client.wait_for_workers(10)
        client.restart()
-        yield client
+        with coiled.performance_report(f"{request.node.originalname}-{UNIQUE_ID}.html"):


With this, we are storing the performance reports on coiled, but I can't seem to find which is the bucket or storage that contains them. We should also maybe think about how to clean this up.

I see that in the coiled account we have some performance reports but not all of them, not sure exactly what's happening

I can't seem to find which is the bucket or storage that contains them

I chatted with @shughes-uk offline and she mentioned today all performance reports are stored in a centralized S3 bucket that only Coiled staff have access to. I don't think this is a blocker for us, but it is good to know.

We should also maybe think about how to clean this up.

@shughes-uk mentioned this shouldn't become a problem unless we create millions of performance reports. I'm inclined to not worry about this for now -- I think it will take us a while before we've created millions of these : )

I see that in the coiled account we have some performance reports but not all of them, not sure exactly what's happening

@koverholt pointed out that there's currently a limit of 5 hosted performance reports per user. Per the coiled.performance_report API docs

Note each user is limited to 5 hosted reports with each a maximum file size of 10mb.

So if we want to keep performance reports around, then we should upload them to our own S3 bucket

…performance-reports

ian-r-rose · 2022-05-03T19:16:11Z

Depending on what you are looking for, the analytics page might get you this information for free

jrbourbeau · 2022-05-04T19:51:08Z

Depending on what you are looking for, the analytics page might get you this information for free

That's a good point. With both #102 and #79 it would/would have been nice to see a performance report when we notice that something is taking longer than expected. I think this is definitely in the category of "nice to have" but not required.

ian-r-rose · 2022-06-01T20:35:39Z

conftest.py

    with Client(small_cluster) as client:
        small_cluster.scale(10)
        client.wait_for_workers(10)
        client.restart()
-        yield client
+        local_file = str(tmp_path / "preformance-report.html")
+        with performance_report(local_file):


If we use the coiled version of performance reports, we should get hosting for free

That's what I was initially wanting to do, but I believe there's a limit of 5 hosted reports today (xref #107 (comment))

Oof, good point, I had forgotten about that

ncclementi · 2022-06-09T14:48:14Z

@jrbourbeau I added a life-cycle to the s3 prefix (test-scratch/performance-reports) where the files in this "directory" will be removed after 7 days of being added, I haven't found a way of removing them if not accessed (it doesn't seem possible), but I think 7 days is probably enough time to download a file locally if needed.

Would you be ok, would that?

EDIT: Bump this period to 30 days

…performance-reports

ncclementi · 2022-06-09T21:40:04Z

I'm not seeing any new performance report being uploaded to s3, the latest date is 05/27.
I think that some of the changes might have affected this, even though things are not failing...

ncclementi · 2022-06-09T21:46:07Z

conftest.py

+        local_file = str(tmp_path / "preformance-report.html")
+        with performance_report(local_file):
+            yield client
+        s3.put(local_file, s3_report_url + f"/{request.node.originalname}.html")


Do we need to pass any credentials here? I can't find the proper docs, but my guess we are not seeing updates as we are not passing credentials

Never mind, we are providing s3 which has the credentials

We pass credentials when we create the s3 instance

Then there has to be another problem because the files are not being uploaded

Yeah, I see what I did wrong. I'll have to push up a commit with a fix, but probably not today

…performance-reports

conftest.py

ncclementi · 2022-06-10T19:59:10Z

Ci It's not finished yet but it seems things are working. I'm seeing performance reports uploaded. I wonder if we should log the link same way we do with cluster dumps, at the moment from CI I can't find where the performance reports are.

This is unrelated, but I'm seeing mismatches warnings. Should we worried about these ones?

…performance-reports

ncclementi · 2022-06-25T00:36:35Z

Current status and problems (showed on this CI run):

live logging to log performance report link as warning doesn't work on CI, even though it works locally. Hence, we can't have a link to performance reports unless we have a failure. The performance report gets uploaded to S3 but no link is logged. Not being able to get a performance report link it makes it impossible to find the performance report on S3.
When tests that rely on small client fixture fail the logging of the performance report works, but only when it fails.
For the case of tests/stability/test_deadlock which uses its own cluster/client I can't make the upload performance report work, it's not even uploading the file, even less will get a link

In conclusion,

It looks like CI is not giving us all the logs and haven't been able to find a way to do so when tests are passing.
For the case of tests/stability/test_deadlock I'm not too worried in getting perf reports to work, although it'll be nice if it did.

cc: @ian-r-rose what do you think we should do here?

jrbourbeau · 2022-08-03T20:54:43Z

@ian-r-rose @ncclementi and I chatted about this offline and decided to put this on hold. This is a nice-to-have but also somewhat complicated. If this becomes nicer-to-have in the future we can revive this PR

Add performance reports

4933e92

ncclementi reviewed May 1, 2022

View reviewed changes

Merge branch 'main' of https://github.com/coiled/coiled-runtime into …

178c3ed

…performance-reports

jrbourbeau self-assigned this May 3, 2022

Upload performance reports

0bb116f

ncclementi added 2 commits May 27, 2022 12:17

Merge branch 'main' into performance-reports

d4bcace

fix isort lint

6c951da

ian-r-rose reviewed Jun 1, 2022

View reviewed changes

jrbourbeau added 3 commits June 9, 2022 12:17

Merge branch 'main' of https://github.com/coiled/coiled-runtime into …

92bf35f

…performance-reports

Make uploading performance report logic reusable

511b539

Make parquet client fixture function scoped

445f629

ncclementi reviewed Jun 9, 2022

View reviewed changes

jrbourbeau added 2 commits June 10, 2022 13:21

Merge branch 'main' of https://github.com/coiled/coiled-runtime into …

ef18152

…performance-reports

Fixup

b389f0e

ncclementi reviewed Jun 10, 2022

View reviewed changes

conftest.py Outdated Show resolved Hide resolved

ncclementi reviewed Jun 10, 2022

View reviewed changes

conftest.py Outdated Show resolved Hide resolved

Undo debugging

30187ec

jrbourbeau added 3 commits June 14, 2022 13:26

Merge branch 'main' of https://github.com/coiled/coiled-runtime into …

a61ef0a

…performance-reports

Add logging

4fa6b33

Rerun CI

ee0cd61

ncclementi self-assigned this Jun 23, 2022

ncclementi added 4 commits June 24, 2022 11:29

try to fix conflicts

1503f23

test logging warn

f5dc265

Force fail on purpose for testing

108dc1c

test logging cli level

96b4cb7

ncclementi added 6 commits June 24, 2022 16:40

try yield perf report

3bd9b52

test dump works without perf in deadlock test

165a737

Merge branch 'main' into performance-reports

6118719

reduce testing until temp

afc3347

try something different in logging

07311cb

show status of the situation

9a5736d

ian-r-rose self-requested a review June 28, 2022 15:47

jrbourbeau closed this Aug 3, 2022

ncclementi mentioned this pull request Aug 26, 2022

Upload performance reports #281

Open

ncclementi mentioned this pull request Oct 3, 2022

Improve traceability between currently-running tests and Coiled resources they create #267

Open

5 tasks

ncclementi deleted the performance-reports branch December 28, 2022 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add performance reports #107

Add performance reports #107

jrbourbeau commented Apr 29, 2022

ncclementi May 1, 2022

jrbourbeau May 3, 2022

jrbourbeau May 3, 2022

ian-r-rose commented May 3, 2022

jrbourbeau commented May 4, 2022

ian-r-rose Jun 1, 2022

jrbourbeau Jun 1, 2022

ian-r-rose Jun 1, 2022

ncclementi commented Jun 9, 2022 •

edited

Loading

ncclementi commented Jun 9, 2022

ncclementi Jun 9, 2022

ncclementi Jun 9, 2022

jrbourbeau Jun 9, 2022

ncclementi Jun 9, 2022

jrbourbeau Jun 9, 2022

ncclementi commented Jun 10, 2022

ncclementi commented Jun 25, 2022 •

edited

Loading

jrbourbeau commented Aug 3, 2022

Add performance reports #107

Add performance reports #107

Conversation

jrbourbeau commented Apr 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ian-r-rose commented May 3, 2022

jrbourbeau commented May 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncclementi commented Jun 9, 2022 • edited Loading

ncclementi commented Jun 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncclementi commented Jun 10, 2022

ncclementi commented Jun 25, 2022 • edited Loading

jrbourbeau commented Aug 3, 2022

ncclementi commented Jun 9, 2022 •

edited

Loading

ncclementi commented Jun 25, 2022 •

edited

Loading