RE2022-217: Workspace uploader performance #426

Xiangs18 · 2023-08-23T00:56:14Z

No description provided.

codecov · 2023-08-23T00:58:25Z

Codecov Report

Attention: 102 lines in your changes are missing coverage. Please review.

Comparison is base (8b48835) 46.97% compared to head (c92fe2f) 47.00%.

Files	Patch %	Lines
...c/loaders/workspace_uploader/workspace_uploader.py	63.20%	46 Missing ⚠️
src/loaders/common/callback_server_wrapper.py	3.44%	28 Missing ⚠️
src/loaders/common/loader_helper.py	20.00%	24 Missing ⚠️
...aders/workspace_downloader/workspace_downloader.py	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #426      +/-   ##
==========================================
+ Coverage   46.97%   47.00%   +0.02%     
==========================================
  Files          75       75              
  Lines        6518     6612      +94     
==========================================
+ Hits         3062     3108      +46     
- Misses       3456     3504      +48

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/loaders/workspace_uploader/workspace_uploader.py

Xiangs18 · 2023-09-05T23:33:12Z

save_assemblies_from_fastas_integration_test.py::test_import_fasta_mass_timing Start uploading to 69141 in test_import_fasta_mass_timing fn
validating parameters
The following files are passed in _import_fasta_mass fn:

GCA_002506415.1_ASM250641v1_genomic.fna.gz
GCF_000007065.1_ASM706v1_genomic.fna.gz
GCF_000970165.1_ASM97016v1_genomic.fna.gz
GCF_000970185.1_ASM97018v1_genomic.fna.gz
GCF_000970205.1_ASM97020v1_genomic.fna.gz
GCF_000970245.1_ASM97024v1_genomic.fna.gz
GCF_000979375.1_gtlEnvA5udCFS_genomic.fna.gz
GCF_000979555.1_gtlEnvA5udCFS_genomic.fna.gz
GCF_000980105.1_gtlEnvA5udCFS_genomic.fna.gz
GCF_000980175.1_gtlEnvA5udCFS_genomic.fna.gz

staged 10 files in 5.6641364097595215 seconds
parsing FASTA file: /kb/module/work/tmp/import_fasta_0955b238-9e96-4095-9172-dddd4dfff0c6/GCA_002506415.1_ASM250641v1_genomic.fna
parsed 130 contigs, 3839682 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_878d8535-ea93-404b-b9a3-d43e67ca5f69/GCF_000007065.1_ASM706v1_genomic.fna
parsed 1 contigs, 4096345 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_b4a1b8e1-2164-4822-8e8e-4bd984d722ca/GCF_000970165.1_ASM97016v1_genomic.fna
parsed 1 contigs, 4096482 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_c05de458-98ad-447f-b3ed-735a2595c73b/GCF_000970185.1_ASM97018v1_genomic.fna
1693955669.0551195: uploading file /kb/module/work/tmp/import_fasta_878d8535-ea93-404b-b9a3-d43e67ca5f69/GCF_000007065.1_ASM706v1_genomic.fna into shock node
1693955672.2739327: uploading done into shock node: d5e6b96b-d3f1-493f-a9b6-c9ba9bfb29b2
1693955672.2740831: uploading file /kb/module/work/tmp/import_fasta_b4a1b8e1-2164-4822-8e8e-4bd984d722ca/GCF_000970165.1_ASM97016v1_genomic.fna into shock node
1693955677.3947856: uploading done into shock node: 6ce559d9-e733-448d-a11e-84dbaaeed99e
parsed 1 contigs, 4066551 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_7135350e-3363-493c-8360-b6c68c7d9b2a/GCF_000970205.1_ASM97020v1_genomic.fna
parsed 1 contigs, 4142816 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_65828b11-6633-4270-ab27-985ae2eab967/GCF_000970245.1_ASM97024v1_genomic.fna
parsed 1 contigs, 4166241 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_9da2a649-9a60-42e5-832f-052fcad14ea5/GCF_000979375.1_gtlEnvA5udCFS_genomic.fna
parsed 136 contigs, 4092300 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_fac65d9a-860e-41ad-b63f-0f1fe08f374b/GCF_000979555.1_gtlEnvA5udCFS_genomic.fna
parsed 185 contigs, 4078931 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_259d4045-4384-433f-9ca2-718ac1d61227/GCF_000980105.1_gtlEnvA5udCFS_genomic.fna
parsed 260 contigs, 4079204 bp
parsing FASTA file: /kb/module/work/tmp/import_fasta_23bb941d-b645-41df-9f3f-ff06c174330c/GCF_000980175.1_gtlEnvA5udCFS_genomic.fna
parsed 142 contigs, 4085981 bp
parsed 10 FASTA files in 5.011489391326904 seconds
saving assemblies to KBase
Uploading FASTA files to the Blobstore
1693955663.00 - CallbackServer: Subjob method: DataFileUtil.file_to_shock_mass JobID: 2dbd1842-ff75-44dc-ba4c-705610b5fda6
node
1693955699.9393437: uploading done into shock node: 1e25e7bd-6c27-4f07-aabd-fff1df1a9989
1693955699.9401429: uploading file /kb/module/work/tmp/import_fasta_259d4045-4384-433f-9ca2-718ac1d61227/GCF_000980105.1_gtlEnvA5udCFS_genomic.1693955663.00 - CallbackServer: WARNING: Module DataFileUtil was already used once for this job. Using cached version:
url: https://github.com/kbaseapps/DataFileUtil
node
1693955699.9393437: uploading done into shock node: 1e25e7bd-6c27-4f07-aabd-fff1df1a9989
1693955699.9401429: uploading file /kb/module/work/tmp/import_fasta_259d4045-4384-433f-9ca2-718ac1d61227/GCF_000980105.1_gtlEnvA5udCFS_genomic.1693955699.9401429: uploading file /kb/module/work/tmp/import_fasta_259d4045-4384-433f-9ca2-718ac1d61227/GCF_000980105.1_gtlEnvA5udCFS_genomic.fna into shock node
1693955703.332774: uploading done into shock node: c243eeee-63c7-45b1-bb37-b2f3efdce2ee
1693955703.3329513: uploading file /kb/module/work/tmp/import_fasta_23bb941d-b645-41df-9f3f-ff06c174330c/GCF_000980175.1_gtlEnvA5udCFS_genomic.fna into shock node
1693955705.995946: uploading done into shock node: a62d7640-2cd1-46e6-93ca-d3b9c785dff0
saved 10 files to the Blobstore in 58.62708044052124 seconds
built 10 assembly objects in 0.15749096870422363 seconds
Saving Assemblies to Workspace
1693955721.81 - CallbackServer: Subjob method: DataFileUtil.save_objects JobID: 67f4e667-ffae-415f-8462-632141a7d8a9
1693955721.81 - CallbackServer: WARNING: Module DataFileUtil was already used once for this job. Using cached version:
url: https://github.com/kbaseapps/DataFileUtil
commit: 6268e0f87425cdd96901299e618d6778aa44425a
version: 0.2.0
release: release

1693955723.5434816: Shock url: https://ci.kbase.us/services/shock-api

saved 10 assemblies to the Workspace in 5.342104434967041 seconds
PASSEDDeleted shock node 784cdab7-3bdd-41d7-a09b-353c38c9f752
Deleted shock node d5e6b96b-d3f1-493f-a9b6-c9ba9bfb29b2
Deleted shock node 6ce559d9-e733-448d-a11e-84dbaaeed99e
Deleted shock node 5e46fbd1-2522-4de7-8c87-95234efa2246
Deleted shock node edb2e4aa-ef0a-4b10-87d5-75970d36c436
Deleted shock node cf4dc0d1-3ab0-4595-a285-82177ddbac77
Deleted shock node ec866e05-9337-4e12-8fc1-0bd526f4f9c5
Deleted shock node 1e25e7bd-6c27-4f07-aabd-fff1df1a9989
Deleted shock node c243eeee-63c7-45b1-bb37-b2f3efdce2ee
Deleted shock node a62d7640-2cd1-46e6-93ca-d3b9c785dff0

----------- coverage: platform linux, python 3.6.3-final-0 -----------
Coverage HTML written to dir htmlcov
Coverage XML written to file coverage.xml

========================= 1 passed in 83.58s (0:01:23) =========================
Shutting down callback server...
1693955730.77 - CallbackServer: Shutting down executor service

Xiangs18 · 2023-09-05T23:36:08Z

I highlight the key time stats from the console via running kb-sdk test on _import_fasta_mass function:

staged 10 files in 5.6641364097595215 seconds
parsed 10 FASTA files in 5.011489391326904 seconds
saved 10 files to the Blobstore in 58.62708044052124 seconds
built 10 assembly objects in 0.15749096870422363 seconds
saved 10 assemblies to the Workspace in 5.342104434967041 seconds

Xiangs18 · 2023-11-21T02:18:27Z

@MrCreosote @Tianhao-Gu
Update since last code review:

Delete all queue stuff and move batching logic && parallelization to AssemblyUtil
Use a list of nameTuples as suggested to avoid buggy code
Add 2 new CLI: jr_max_tasks and au_service _ver
Add containers.conf file setup so we can get logs from the callback server
Fix catalog params (Catalog params fix kbaseapps/AssemblyUtil#91)
Rename, relocate, and update the callback server wrapper (rename & relocate workspace_downloader_helper.py file #555)

src/loaders/workspace_uploader/workspace_uploader_helper.py

src/loaders/common/loader_helper.py

src/loaders/workspace_uploader/workspace_uploader.py

MrCreosote

Mostly minor stuff in this review I think

Still need to review tests, but I'll wait until the code is done

I'll be really glad when we merge this PR, it takes a long time to review...

src/loaders/workspace_uploader/workspace_uploader.py

MrCreosote · 2024-01-26T00:21:04Z

Note to self: all my comments above this point have been addressed

MrCreosote

Ok, went thought the tests finally

src/loaders/workspace_uploader/workspace_uploader.py

test/src/loaders/workspace_uploader/workspace_uploader_test.py

Xiangs18 · 2024-01-30T01:37:22Z

Addressed all comments in the tests. We are getting really close to merge this PR. Maybe after Podman test ...

MrCreosote

yeah, looking about done. I think there are 2 things left beyond the fairly trivial comment in this review:

Test the code with podman, including the recovery code, now that it's fixed
@Tianhao-Gu should take another look as the PR has been basically completely rewritten since his review and I've looked at it so many times now for so long I've probably lost objectivity

test/src/loaders/workspace_uploader/workspace_uploader_test.py

Xiangs18 · 2024-01-31T19:30:06Z

@MrCreosote Tested the uploader with podman, including the recovery code. Everything looks good!

Tianhao-Gu

Overall looks good except some typos. Are we planning to refactor it and upload genome all together in the future? Currently, workspace downloader is expecting every Assembly has an associated Genome obj.

Tianhao-Gu · 2024-01-31T20:42:39Z

src/loaders/common/callback_server_wrapper.py

                                If not supplied, the token must be provided in the environment variable KB_AUTH_TOKEN.
+                                The KB_ADMIN_AUTH_TOKEN environment variable will get set by this token if the user runs as catalog admin.
+            au_service_ver (str): The service verison of AssemblyUtilClient


Suggested change

au_service_ver (str): The service verison of AssemblyUtilClient

au_service_ver (str): The service version of AssemblyUtilClient

Tianhao-Gu · 2024-01-31T20:43:16Z

src/loaders/common/callback_server_wrapper.py

            workers (int): The number of workers to use for multiprocessing.
+            max_callback_server_tasks (int): The maxmium subtasks for the callback server.


Suggested change

max_callback_server_tasks (int): The maxmium subtasks for the callback server.

max_callback_server_tasks (int): The maximum number of subtasks for the callback server.

src/loaders/workspace_uploader/workspace_uploader.py

Tianhao-Gu · 2024-01-31T22:07:08Z

src/loaders/workspace_uploader/workspace_uploader.py

+        "--cbs_max_tasks",
+        type=int,
+        default=20,
+        help="The maxmium subtasks for the callback server",


Suggested change

help="The maxmium subtasks for the callback server",

help="The maximum subtasks for the callback server",

MrCreosote · 2024-01-31T22:09:21Z

Are we planning to refactor it and upload genome all together in the future?

yep

Currently, workspace downloader is expecting every Assembly has an associated Genome obj.

There's no toggle to choose to d/l genomes or not?

Tianhao-Gu · 2024-01-31T22:16:31Z

There's no toggle to choose to d/l genomes or not?

As far as I understand, no option to ignore genome existence. Looks to me, it doesn't really download the genome obj. It just saves the genome_upa to the meta file.

MrCreosote · 2024-01-31T22:20:42Z

Yeah, it needs that for exporting genome sets. There's no real need for the genome data proper in the pipeline as yet

Xiangs18 added 3 commits August 21, 2023 17:57

add dynamic batch_size upload

d215fd7

add comments for iterator and fix a bug

1524c47

finish clean up && testing

f1ac0b4

Xiangs18 requested review from MrCreosote and Tianhao-Gu August 23, 2023 00:56

Tianhao-Gu reviewed Aug 23, 2023

View reviewed changes

Xiangs18 added 2 commits August 23, 2023 13:41

correct tuple type hint

48df915

Merge branch 'main' into dv-WS_uploader_performance

f0fdadf

MrCreosote reviewed Aug 23, 2023

View reviewed changes

Xiangs18 added 12 commits October 11, 2023 10:29

Merge branch 'main' into dv-WS_uploader_performance

66bb799

Merge branch 'main' into dv-WS_uploader_performance

0f07b4a

update WS uploader script

1eea48f

Merge branch 'main' into dv-WS_uploader_performance

51a9225

remove workers and run pass all unit tests

8117061

Merge branch 'main' into dv-WS_uploader_performance

95d48c4

Merge branch 'main' into dv-WS_uploader_performance

7bf05b8

add AU service_ver CLI

19e52ae

print out logs and add env max_task

2383f1b

container.conf setup && clean up

d305518

update setup_callback_server_logs fun

20673cd

Merge branch 'main' into dv-WS_uploader_performance

dc20206

fix catalog param

5972e2d

MrCreosote reviewed Nov 27, 2023

View reviewed changes

Xiangs18 added 2 commits November 27, 2023 18:27

update _upload_assembly_files_in_parallel logic && logs permission

d71c661

test passed new logic

6ae5f4d

Xiangs18 mentioned this pull request Nov 29, 2023

Callback server wrapper #554

Closed

MrCreosote reviewed Dec 22, 2023

View reviewed changes

Xiangs18 added 6 commits January 19, 2024 15:49

fix logic in workspace_uploader.py only

d0f4d45

fix all the tests

744fe56

Merge branch 'main' into dv-WS_uploader_performance

70def6a

fix format

c64ce1d

fix race condition on containers.conf

e3a9b4c

add --as_catalog_admin CLI param

51d00da

MrCreosote reviewed Jan 24, 2024

View reviewed changes

src/loaders/workspace_uploader/workspace_uploader.py Show resolved Hide resolved

Xiangs18 added 2 commits January 24, 2024 17:13

update CLI help message && file lock

1b09e3f

update description && add try finally block for file lock

5caae42

MrCreosote reviewed Jan 29, 2024

View reviewed changes

Xiangs18 added 2 commits January 29, 2024 16:54

fix tests

9a91857

Merge branch 'main' into dv-WS_uploader_performance

b75e0e1

MrCreosote reviewed Jan 31, 2024

View reviewed changes

test/src/loaders/workspace_uploader/workspace_uploader_test.py Outdated Show resolved Hide resolved

Xiangs18 added 2 commits January 30, 2024 17:14

remove redudant code

a412797

Merge branch 'main' into dv-WS_uploader_performance

779ae79

Tianhao-Gu previously approved these changes Jan 31, 2024

View reviewed changes

Tianhao-Gu reviewed Jan 31, 2024

View reviewed changes

correct typos

c92fe2f

Xiangs18 dismissed Tianhao-Gu’s stale review via c92fe2f January 31, 2024 23:58

Tianhao-Gu approved these changes Feb 1, 2024

View reviewed changes

Xiangs18 merged commit 86ddf57 into main Feb 1, 2024
10 of 11 checks passed

Xiangs18 deleted the dv-WS_uploader_performance branch February 1, 2024 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RE2022-217: Workspace uploader performance #426

RE2022-217: Workspace uploader performance #426

Xiangs18 commented Aug 23, 2023

codecov bot commented Aug 23, 2023 •

edited

Loading

Xiangs18 commented Sep 5, 2023

Xiangs18 commented Sep 5, 2023 •

edited

Loading

Xiangs18 commented Nov 21, 2023 •

edited

Loading

MrCreosote left a comment

MrCreosote commented Jan 26, 2024

MrCreosote left a comment

Xiangs18 commented Jan 30, 2024 •

edited

Loading

MrCreosote left a comment

Xiangs18 commented Jan 31, 2024

Tianhao-Gu left a comment •

edited

Loading

Tianhao-Gu Jan 31, 2024

Xiangs18 Jan 31, 2024

Tianhao-Gu Jan 31, 2024

Xiangs18 Jan 31, 2024

Tianhao-Gu Jan 31, 2024

Xiangs18 Jan 31, 2024

MrCreosote commented Jan 31, 2024

Tianhao-Gu commented Jan 31, 2024 •

edited

Loading

MrCreosote commented Jan 31, 2024

	au_service_ver (str): The service verison of AssemblyUtilClient
	au_service_ver (str): The service version of AssemblyUtilClient

		workers (int): The number of workers to use for multiprocessing.
		max_callback_server_tasks (int): The maxmium subtasks for the callback server.

	help="The maxmium subtasks for the callback server",
	help="The maximum subtasks for the callback server",

RE2022-217: Workspace uploader performance #426

RE2022-217: Workspace uploader performance #426

Conversation

Xiangs18 commented Aug 23, 2023

codecov bot commented Aug 23, 2023 • edited Loading

Codecov Report

Xiangs18 commented Sep 5, 2023

Xiangs18 commented Sep 5, 2023 • edited Loading

Xiangs18 commented Nov 21, 2023 • edited Loading

MrCreosote left a comment

Choose a reason for hiding this comment

MrCreosote commented Jan 26, 2024

MrCreosote left a comment

Choose a reason for hiding this comment

Xiangs18 commented Jan 30, 2024 • edited Loading

MrCreosote left a comment

Choose a reason for hiding this comment

Xiangs18 commented Jan 31, 2024

Tianhao-Gu left a comment • edited Loading

Choose a reason for hiding this comment

Tianhao-Gu Jan 31, 2024

Choose a reason for hiding this comment

Xiangs18 Jan 31, 2024

Choose a reason for hiding this comment

Tianhao-Gu Jan 31, 2024

Choose a reason for hiding this comment

Xiangs18 Jan 31, 2024

Choose a reason for hiding this comment

Tianhao-Gu Jan 31, 2024

Choose a reason for hiding this comment

Xiangs18 Jan 31, 2024

Choose a reason for hiding this comment

MrCreosote commented Jan 31, 2024

Tianhao-Gu commented Jan 31, 2024 • edited Loading

MrCreosote commented Jan 31, 2024

codecov bot commented Aug 23, 2023 •

edited

Loading

Xiangs18 commented Sep 5, 2023 •

edited

Loading

Xiangs18 commented Nov 21, 2023 •

edited

Loading

Xiangs18 commented Jan 30, 2024 •

edited

Loading

Tianhao-Gu left a comment •

edited

Loading

Tianhao-Gu commented Jan 31, 2024 •

edited

Loading