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
Show file tree
Hide file tree
Changes from 1 commit
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
21 changes: 15 additions & 6 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,13 @@ def small_cluster(request):


@pytest.fixture
def small_client(small_cluster, s3, s3_report_url, request, tmp_path):
def small_client(small_cluster, upload_performance_report):
with Client(small_cluster) as client:
small_cluster.scale(10)
client.wait_for_workers(10)
client.restart()
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")
yield client
upload_performance_report(client)


S3_REGION = "us-east-2"
Expand Down Expand Up @@ -133,7 +131,7 @@ def s3_scratch(s3):

@pytest.fixture(scope="session")
def s3_report_url(s3, s3_scratch):
# Ensure that the test-scratch directory exists,
# Ensure that the performance-reports 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}"
Expand All @@ -147,3 +145,14 @@ def s3_url(s3, s3_scratch, request):
s3.mkdirs(url, exist_ok=False)
yield url
s3.rm(url, recursive=True)


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

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, we are providing s3 which has the credentials

Copy link
Member Author

Choose a reason for hiding this comment

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

We pass credentials when we create the s3 instance

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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


return func
3 changes: 2 additions & 1 deletion tests/benchmarks/test_parquet.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ def parquet_cluster():


@pytest.fixture(scope="module")
def parquet_client(parquet_cluster):
def parquet_client(parquet_cluster, upload_performance_report):
with distributed.Client(parquet_cluster) as client:
parquet_cluster.scale(N_WORKERS)
client.wait_for_workers(N_WORKERS)
client.restart()
yield client
upload_performance_report(client)


def test_read_spark_generated_data(parquet_client):
Expand Down