Skip to content

CMR-11560: V2 Fix Large Collection Delete - #2535

Open
jmaeng72 wants to merge 51 commits into
masterfrom
CMR-11560-v2
Open

jmaeng72 wants to merge 51 commits into
masterfrom
CMR-11560-v2

Conversation

@jmaeng72

@jmaeng72 jmaeng72 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Overview

What is the objective?

To let large collections be deleted properly without timeout or server errors

What are the changes?

NOTE: This was reverted a few days ago but is now back ready for PR.

Since the last PR, the only thing I changed was the default scroll-size, requests-per-sec and slices of the cascade collection delete

  • Created new index delete queue
  • Created new ingest delete exchange
  • Collection deletes (to delete its granules) routed to new exchange and queue (other concept deletes will remain going to original index queue and ingest exchange)
  • Collection cascade delete in DB is now async and runs in background
  • Race condition of users re-creating collection and grans while a delete in happening background is addressed by tracking timestamp of delete request and re-created coll and grans will not be deleted
  • Collection cascade delete in ES is now async and runs in background with longer SQS timeout and will only run one delete task at a time

What areas of the application does this impact?

Cascade collection delete - ingest, indexer, metadata-db

Required Checklist

  • New and existing unit and int tests pass locally and remotely
  • clj-kondo has been run locally and all errors in changed files are corrected
  • I have commented my code, particularly in hard-to-understand areas
  • I have made changes to the documentation (if necessary)
  • My changes generate no new warnings

Additional Checklist

  • I have removed unnecessary/dead code and imports in files I have changed
  • I have cleaned up integration tests by doing one or more of the following:
    • migrated any are2 tests to are3 in files I have changed
    • de-duped, consolidated, removed dead int tests
    • transformed applicable int tests into unit tests
    • reduced number of system state resets by updating fixtures. Ex) (use-fixtures :each (ingest/reset-fixture {})) to be :once instead of :each

{:default 200
:type Long})

(defconfig force-delete-concept-by-params-db-batch-size

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

new change since last PR

"auto"
(Long/parseLong val)))
:scroll-size (config/collection-delete-elastic-scroll-size)
:requests-per-second (config/collection-delete-elastic-requests-per-second)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

new change since last PR

{:default false
:type Boolean})

(defconfig collection-delete-elastic-slices

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

new change since last PR

:revision-date :transaction-id)
(dissoc deleted-coll :revision-date :transaction-id))))

(defn- wait-until-granule-revisions-deleted

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the updated delete test that @daniel-zamora wrote that was needed to be paired with the async coll delete changes in this PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(is (not (mdb/concept-exists-in-mdb? (:concept-id gran1) (:revision-id gran1)))

Can you check here as well?

{:error error-code :error-message error-message :throwable e})))]

;; Only run the post-commit-step if the above save was 100% successful aka returned nil
(when (nil? save-result)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could the background delete start from try-to-save once the transaction-id check passes? If that check rolls the tombstone back with a 409, granules of a still-live collection may already be getting deleted.

;; Only run the post-commit-step if the above save was 100% successful aka returned nil
(when (nil? save-result)
;; start a separate thread to do the post-commit-step db actions
(future

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it possible to run this too many times and use up the Oracle connection pool if we're deleting many large collections at once, for example through cleanup-expired-collections or a provider scripting deletes?

(post-commit-step db provider concept)

(catch Exception e
(error e (format "ERROR - Background Oracle background process save-concept failed in post-commit-step for concept %s" (:concept-id concept)))))))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this throws or metadata-db restarts mid-loop, the remaining granules stay under the tombstone and nothing picks them up again. Do we care about that?

(Long/parseLong val)))
:scroll-size (config/collection-delete-elastic-scroll-size)
:requests-per-second (config/collection-delete-elastic-requests-per-second)
:max-wait-ms (* (config/index-delete-queue-max-wait-secs) 1000)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are there any pathways to this function that wouldn't want this max-wait-ms?

:revision-date :transaction-id)
(dissoc deleted-coll :revision-date :transaction-id))))

(defn- wait-until-granule-revisions-deleted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(is (not (mdb/concept-exists-in-mdb? (:concept-id gran1) (:revision-id gran1)))

Can you check here as well?

(let [e (second result)
ex-data-map (ex-data e)]

(if (and (< attempt max-attempts) (= :scroll-context-error (:type ex-data-map)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it ever possible to wait too long, and have SQS redeliver the collection delete message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants