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

APPS-2549 Bulk describe optimization in scatter-collect to cover files in arrays, structs #464

Merged
merged 35 commits into from
Mar 25, 2024

Conversation

r-i-v-a
Copy link
Member

@r-i-v-a r-i-v-a commented Mar 19, 2024

@r-i-v-a r-i-v-a added the integration2 Integration tests with dxcint label Mar 19, 2024
@r-i-v-a r-i-v-a removed the integration2 Integration tests with dxcint label Mar 20, 2024
@r-i-v-a r-i-v-a changed the title APPS-2549 Investigate caching /file/describe APPS-2549 Bulk describe optimization in scatter-collect to cover files in arrays, structs Mar 21, 2024
@@ -414,7 +418,6 @@ abstract class JobMeta(val workerPaths: DxWorkerPaths,
if (queryFiles.isEmpty) {
queryFiles
} else {
logger.trace(s"Bulk describing ${queryFiles.size} files")
Copy link
Member Author

Choose a reason for hiding this comment

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

This logging is moved to dxApi (other PR).

} collect {
case dxFile: DxFile => dxFile
} flatMap {
dxApi.flattenDxFileObjectsFromJson
Copy link
Member Author

Choose a reason for hiding this comment

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

The part using the new implementation from dxApi

@r-i-v-a r-i-v-a marked this pull request as ready for review March 22, 2024 00:28
@r-i-v-a r-i-v-a requested a review from Gvaihir March 22, 2024 00:28
Copy link
Contributor

@Gvaihir Gvaihir left a comment

Choose a reason for hiding this comment

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

Looks good! In the ticket please:

  1. Run a WF prior to changes, and provide a splunk stats showing file/describe from dxScala agent. Should be some.
  2. Run after changes applied showing splunk stats for the same WF (new analysis) and decreased/eliminated dxScala-made file/describe.

@r-i-v-a r-i-v-a added the integration2 Integration tests with dxcint label Mar 22, 2024
@r-i-v-a
Copy link
Member Author

r-i-v-a commented Mar 22, 2024

Looks good! In the ticket please:

  1. Run a WF prior to changes, and provide a splunk stats showing file/describe from dxScala agent. Should be some.
  2. Run after changes applied showing splunk stats for the same WF (new analysis) and decreased/eliminated dxScala-made file/describe.

done -- APPS-2549

@r-i-v-a r-i-v-a added the cwl_conformance Triggers CWL conformance tools and workflows tests label Mar 22, 2024
@r-i-v-a r-i-v-a requested a review from Gvaihir March 22, 2024 15:02
@r-i-v-a r-i-v-a merged commit c330a93 into develop Mar 25, 2024
6 checks passed
@r-i-v-a r-i-v-a deleted the APPS-2549_cache_file_describe branch March 25, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cwl_conformance Triggers CWL conformance tools and workflows tests integration2 Integration tests with dxcint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants