Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add performance reports #107

Closed
wants to merge 24 commits into from
Closed
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4933e92
Add performance reports
jrbourbeau Apr 29, 2022
178c3ed
Merge branch 'main' of https://github.com/coiled/coiled-runtime into …
jrbourbeau May 3, 2022
0bb116f
Upload performance reports
jrbourbeau May 4, 2022
d4bcace
Merge branch 'main' into performance-reports
ncclementi May 27, 2022
6c951da
fix isort lint
ncclementi May 27, 2022
92bf35f
Merge branch 'main' of https://github.com/coiled/coiled-runtime into …
jrbourbeau Jun 9, 2022
511b539
Make uploading performance report logic reusable
jrbourbeau Jun 9, 2022
445f629
Make parquet client fixture function scoped
jrbourbeau Jun 9, 2022
ef18152
Merge branch 'main' of https://github.com/coiled/coiled-runtime into …
jrbourbeau Jun 10, 2022
b389f0e
Fixup
jrbourbeau Jun 10, 2022
30187ec
Undo debugging
jrbourbeau Jun 10, 2022
a61ef0a
Merge branch 'main' of https://github.com/coiled/coiled-runtime into …
jrbourbeau Jun 14, 2022
4fa6b33
Add logging
jrbourbeau Jun 14, 2022
ee0cd61
Rerun CI
jrbourbeau Jun 16, 2022
1503f23
try to fix conflicts
ncclementi Jun 24, 2022
f5dc265
test logging warn
ncclementi Jun 24, 2022
108dc1c
Force fail on purpose for testing
ncclementi Jun 24, 2022
96b4cb7
test logging cli level
ncclementi Jun 24, 2022
3bd9b52
try yield perf report
ncclementi Jun 24, 2022
165a737
test dump works without perf in deadlock test
ncclementi Jun 24, 2022
6118719
Merge branch 'main' into performance-reports
ncclementi Jun 24, 2022
afc3347
reduce testing until temp
ncclementi Jun 24, 2022
07311cb
try something different in logging
ncclementi Jun 24, 2022
9a5736d
show status of the situation
ncclementi Jun 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import dask
import pytest
import s3fs
from dask.distributed import Client
from dask.distributed import Client, performance_report
from toolz import merge

try:
Expand Down Expand Up @@ -43,6 +43,9 @@ def pytest_collection_modifyitems(config, items):
item.add_marker(skip_latest)


UNIQUE_ID = uuid.uuid4().hex[:8]


def get_software():
try:
return os.environ["COILED_SOFTWARE_NAME"]
Expand Down Expand Up @@ -79,7 +82,7 @@ def small_cluster(request):
)
module = os.path.basename(request.fspath).split(".")[0]
with Cluster(
name=f"{module}-{uuid.uuid4().hex[:8]}",
name=f"{module}-{UNIQUE_ID}",
n_workers=10,
worker_memory="8 GiB",
worker_vm_types=["m5.large"],
Expand All @@ -90,12 +93,15 @@ def small_cluster(request):


@pytest.fixture
def small_client(small_cluster):
def small_client(small_cluster, s3, s3_report_url, request, tmp_path):
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):
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oof, good point, I had forgotten about that

yield client
s3.put(local_file, s3_report_url + f"/{request.node.originalname}.html")


S3_REGION = "us-east-2"
Expand Down Expand Up @@ -126,9 +132,19 @@ def s3_scratch(s3):
return scratch_url


@pytest.fixture(scope="session")
def s3_report_url(s3, s3_scratch):
# Ensure that the test-scratch directory exists,
# but do NOT remove it as multiple test runs could be
# accessing it at the same time
report_url = f"{s3_scratch}/performance-reports/{UNIQUE_ID}"
s3.mkdirs(report_url, exist_ok=True)
return report_url


@pytest.fixture(scope="function")
def s3_url(s3, s3_scratch, request):
url = f"{s3_scratch}/{request.node.originalname}-{uuid.uuid4().hex}"
url = f"{s3_scratch}/{request.node.originalname}-{UNIQUE_ID}"
s3.mkdirs(url, exist_ok=False)
yield url
s3.rm(url, recursive=True)