diff --git a/CHANGELOG.md b/CHANGELOG.md index 5842eafdd..2b66a9587 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# Release 0.18.4 +- [GH-1666] Pick up GDCWholeGenomeSomaticSingleSample_v1.3.0 in sg module. ([#610](https://github.com/broadinstitute/wfl/pull/610)) +- [GH-1654] Admit memory retry multiplier. ([#608](https://github.com/broadinstitute/wfl/pull/608)) + # Release 0.18.3 - [GH-1658] Shouldn't wfl.service.rawls/batch-upsert handle Boolean attributes? ([#607](https://github.com/broadinstitute/wfl/pull/607)) - [GH-1653] TDR snapshot direct links have changed ([#606](https://github.com/broadinstitute/wfl/pull/606)) diff --git a/api/src/wfl/executor.clj b/api/src/wfl/executor.clj index b97a509c4..53961bf18 100644 --- a/api/src/wfl/executor.clj +++ b/api/src/wfl/executor.clj @@ -80,6 +80,7 @@ (def ^:private ^:const terra-executor-table "TerraExecutor") (def ^:private ^:const terra-executor-serialized-fields {:workspace :workspace + :memoryRetryMultiplier :memory_retry_multiplier :methodConfiguration :method_configuration :methodConfigurationVersion :method_configuration_version :fromSource :from_source}) @@ -87,6 +88,7 @@ ;; Specs (s/def ::entity ::all/uuid) (s/def ::fromSource string?) +(s/def ::memoryRetryMultiplier number?) (s/def ::methodConfiguration (s/and string? util/terra-namespaced-name?)) (s/def ::methodConfigurationVersion integer?) (s/def ::reference ::all/uuid) @@ -97,7 +99,8 @@ ::fromSource ::methodConfiguration ::all/workspace] - :opt-un [::methodConfigurationVersion])) + :opt-un [::memoryRetryMultiplier + ::methodConfigurationVersion])) (s/def ::terra-executor-workflow (s/keys :req-un [::entity ::methodConfiguration @@ -248,16 +251,16 @@ user-comment-note reference [_type snapshot :as _source-object]] - (let [{:keys [workspace methodConfiguration]} executor] + (let [{:keys [memoryRetryMultiplier methodConfiguration workspace]} executor] (update-method-configuration! workload reference snapshot) (log/debug "Initiating Terra submission" - :workload (workloads/to-log workload) - :methodConfiguration methodConfiguration - :workspace workspace) + :workload (workloads/to-log workload) + :executor executor) (let [userComment (create-user-comment user-comment-note workload snapshot) submission - (firecloud/submit-method workspace methodConfiguration userComment) + (firecloud/submit-method memoryRetryMultiplier methodConfiguration + userComment workspace) message (submission-created-slack-msg executor submission snapshot)] (slack/notify-watchers workload message) diff --git a/api/src/wfl/module/sg.clj b/api/src/wfl/module/sg.clj index d3252c76b..381d48891 100644 --- a/api/src/wfl/module/sg.clj +++ b/api/src/wfl/module/sg.clj @@ -31,7 +31,7 @@ ::all/input_cram])) (def workflow-wdl "The top-level WDL file and its version." - {:release "GDCWholeGenomeSomaticSingleSample_v1.1.0" + {:release "GDCWholeGenomeSomaticSingleSample_v1.3.0" :path "pipelines/broad/dna_seq/somatic/single_sample/wgs/gdc_genome/GDCWholeGenomeSomaticSingleSample.wdl"}) (defn ^:private cromwell->strings diff --git a/api/src/wfl/service/firecloud.clj b/api/src/wfl/service/firecloud.clj index 3962ede03..40eed660b 100644 --- a/api/src/wfl/service/firecloud.clj +++ b/api/src/wfl/service/firecloud.clj @@ -86,21 +86,23 @@ (create-submission workspace methodconfig entity {}))) (defn submit-method - "Submit the`methodconfig` for processing in the Terra `workspace`." - ([workspace methodconfig] - (submit-method workspace methodconfig "")) - ([workspace methodconfig usercomment] - {:pre [(every? string? [workspace methodconfig usercomment])]} - (let [[mcns mcn] (str/split methodconfig #"/")] + "Submit the`methodConfiguration` for processing in the Terra `workspace` + with `memoryRetryMultiplier` when not nil." + ([memoryRetryMultiplier methodConfiguration workspace] + (submit-method memoryRetryMultiplier methodConfiguration workspace "")) + ([memoryRetryMultiplier methodConfiguration workspace userComment] + {:pre [(every? string? [methodConfiguration userComment workspace])]} + (let [[mcns mcn] (str/split methodConfiguration #"/") + body (util/unnilify + {:memoryRetryMultiplier memoryRetryMultiplier + :methodConfigurationNamespace mcns + :methodConfigurationName mcn + :useCallCache true + :userComment userComment})] (-> (workspace-api-url workspace "submissions") (http/post {:headers (auth/get-auth-header) :content-type :application/json - :body (json/write-str - {:methodConfigurationNamespace mcns - :methodConfigurationName mcn - :useCallCache true - :userComment usercomment} - :escape-slash false)}) + :body (json/write-str body :escape-slash false)}) util/response-body-json)))) (defn create-workspace diff --git a/api/test/wfl/integration/executor_test.clj b/api/test/wfl/integration/executor_test.clj index 34c6fd898..e08bbecce 100644 --- a/api/test/wfl/integration/executor_test.clj +++ b/api/test/wfl/integration/executor_test.clj @@ -15,23 +15,28 @@ (:import [java.util UUID] [wfl.util UserException])) -(def ^:private testing-namespace "wfl-dev") -(def ^:private testing-workspace (str testing-namespace "/" "CDC_Viral_Sequencing")) -(def ^:private testing-method-name "sarscov2_illumina_full") -(def ^:private testing-method-configuration (str testing-namespace "/" testing-method-name)) +(def ^:private testing-namespace + "wfl-dev") +(def ^:private testing-workspace + (str testing-namespace "/" "CDC_Viral_Sequencing")) +(def ^:private testing-method-name + "sarscov2_illumina_full") +(def ^:private testing-method-configuration + (str testing-namespace "/" testing-method-name)) (def ^:private testing-method-configuration-version 2) -(let [new-env {"WFL_FIRECLOUD_URL" "https://api.firecloud.org" - "WFL_RAWLS_URL" "https://rawls.dsde-prod.broadinstitute.org"}] - (use-fixtures :once - (fixtures/temporary-environment new-env) - fixtures/temporary-postgresql-database)) +(use-fixtures :once + (fixtures/temporary-environment + {"WFL_FIRECLOUD_URL" "https://api.firecloud.org" + "WFL_RAWLS_URL" "https://rawls.dsde-prod.broadinstitute.org"}) + fixtures/temporary-postgresql-database) (deftest test-validate-terra-executor-with-valid-executor-request - (let [request {:name "Terra" - :workspace testing-workspace - :methodConfiguration testing-method-configuration - :fromSource "importSnapshot"}] + (let [memoryRetryMultiplier 5.23 + request {:name "Terra" + :workspace testing-workspace + :methodConfiguration testing-method-configuration + :fromSource "importSnapshot"}] (letfn [(check-mc-version [executor] (is executor) (is (== testing-method-configuration-version @@ -43,7 +48,12 @@ (testing "request ignores specified method configuration version" (-> (assoc request :methodConfigurationVersion -1) (#'executor/terra-executor-validate-request-or-throw) - check-mc-version))))) + check-mc-version)) + (testing "request propagates memoryRetryMultiplier" + (-> request + (assoc :memoryRetryMultiplier memoryRetryMultiplier) + (#'executor/terra-executor-validate-request-or-throw) + :memoryRetryMultiplier (== memoryRetryMultiplier) is))))) (deftest test-validate-terra-executor-with-invalid-executor-request (is (thrown-with-msg? @@ -200,11 +210,12 @@ (defn ^:private create-terra-executor [id] (jdbc/with-db-transaction [tx (postgres/wfl-db-config)] - (->> {:name "Terra" - :workspace "workspace-ns/workspace-name" - :methodConfiguration fake-method-config - :fromSource "importSnapshot" - :skipValidation true} + (->> {:fromSource "importSnapshot" + :memoryRetryMultiplier 1.23 + :methodConfiguration fake-method-config + :name "Terra" + :skipValidation true + :workspace "workspace-ns/workspace-name"} (executor/create-executor! tx id) (zipmap [:executor_type :executor_items]) (executor/load-executor! tx)))) @@ -246,7 +257,7 @@ (jdbc/with-db-transaction [tx (postgres/wfl-db-config)] (let [[running-record succeeded-record & _ :as records] (->> executor :details (postgres/get-table tx) (sort-by :id)) - executor-record + {:keys [memory_retry_multiplier method_configuration_version]} (#'postgres/load-record-by-id! tx "TerraExecutor" (:id executor)) running-workflow (running-workflow-from-submission init-submission-id) succeeded-workflow (succeeded-workflow-from-submission init-submission-id)] @@ -263,8 +274,9 @@ (is (not (stage/done? executor)) "executor should not have finished processing") (verify-record-against-workflow running-record running-workflow 1) (verify-record-against-workflow succeeded-record succeeded-workflow 2) - (is (== method-config-version-post-update (:method_configuration_version executor-record)) - "Method configuration version was not incremented.")))))) + (is (== method-config-version-post-update method_configuration_version) + "Method configuration version was not incremented.") + (is (== (:memoryRetryMultiplier executor) memory_retry_multiplier))))))) (deftest test-peek-terra-executor-queue (let [succeeded? #{"Succeeded"} diff --git a/api/test/wfl/unit/slack_test.clj b/api/test/wfl/unit/slack_test.clj index d02a054d7..ae2ccf6a1 100644 --- a/api/test/wfl/unit/slack_test.clj +++ b/api/test/wfl/unit/slack_test.clj @@ -20,7 +20,7 @@ (let [queue (conj (PersistentQueue/EMPTY) (make-notification 'test-dispatch-does-not-throw))] (with-redefs - [slack/post-message #({:ok false :error "something_bad"})] + [slack/post-message (constantly {:ok false :error "something_bad"})] (is (= (seq queue) (seq (slack/dispatch-notification queue))) "Queue should remain when posting Slack message returns error")) (with-redefs diff --git a/database/changelog.xml b/database/changelog.xml index 6f9553a93..5cc1955a3 100644 --- a/database/changelog.xml +++ b/database/changelog.xml @@ -49,4 +49,5 @@ + diff --git a/database/changesets/20220523_AddMemoryRetryMultiplierColumn.xml b/database/changesets/20220523_AddMemoryRetryMultiplierColumn.xml new file mode 100644 index 000000000..20c362edb --- /dev/null +++ b/database/changesets/20220523_AddMemoryRetryMultiplierColumn.xml @@ -0,0 +1,23 @@ + + + + + Add support for Cromwell's memoryRetryMultiplier option. + + + ALTER TABLE TerraExecutor + ADD COLUMN memory_retry_multiplier REAL; + + + diff --git a/docs/md/staged-executor.md b/docs/md/staged-executor.md index dc960f11c..49a2d6194 100644 --- a/docs/md/staged-executor.md +++ b/docs/md/staged-executor.md @@ -35,6 +35,7 @@ And a real-life example for a known method configuration: { "name": "Terra", "workspace": "wfl-dev/CDC_Viral_Sequencing", + "memoryRetryMultiplier": 1.5 "methodConfiguration": "wfl-dev/sarscov2_illumina_full", "fromSource": "importSnapshot" } @@ -44,12 +45,13 @@ And a real-life example for a known method configuration: The table below summarises the purpose of each attribute in the above request. -| Attribute | Description | -|-----------------------|---------------------------------------------------------------------------------------------------| -| `name` | Selects the `Terra` executor implementation. | -| `workspace` | Terra Workspace in which to execute workflows. | -| `methodConfiguration` | Method configuration from which to generate submissions. | -| `fromSource` | Instruction to coerce an output from an upstream `Source` to a type understood by this `executor`.| +| Attribute | Description | +|-------------------------|----------------------------------------------------------------------------------------------------| +| `name` | Selects the `Terra` executor implementation. | +| `workspace` | Terra Workspace in which to execute workflows. | +| `memoryRetryMultiplier` | Retry OutOfMemory Cromwell tasks with memory increased by this factor. | +| `methodConfiguration` | Method configuration from which to generate submissions. | +| `fromSource` | Instruction to coerce an output from an upstream `Source` to a type understood by this `executor`. | #### `workspace` A `{workspace-namespace}/{workspace-name}` string as it appears in the URL path diff --git a/version b/version index 267d7e011..0cc988469 100644 --- a/version +++ b/version @@ -1 +1 @@ -0.18.3 +0.18.4