Skip to content

Commit

Permalink
[WIP] Fix Negative History Total After Patient Purge
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderkiel committed Oct 25, 2024
1 parent fe6d9c1 commit 5c61491
Show file tree
Hide file tree
Showing 7 changed files with 311 additions and 26 deletions.
9 changes: 9 additions & 0 deletions .github/scripts/check-resource-totals-after-patient-purge.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash -e

SCRIPT_DIR="$(dirname "$(readlink -f "$0")")"
. "$SCRIPT_DIR/util.sh"

BASE="http://localhost:8080/fhir"
ACTUAL_TOTALS="$(curl -sH 'Accept: application/fhir+json' "$BASE/\$totals" | jq -r '.parameter[] | [.name, .valueUnsignedInt] | @csv')"

test "resource totals" "$ACTUAL_TOTALS" "\"Medication\",1016"
7 changes: 7 additions & 0 deletions .github/scripts/patient-purge-all.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash -e

BASE="http://localhost:8080/fhir"

for ID in $(curl -s "$BASE/Patient?_count=10000&_elements=id" | jq -r '.entry[].resource.id'); do
curl -s -XPOST "$BASE/Patient/$ID/\$purge" -o /dev/null
done
65 changes: 65 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,71 @@ jobs:
- name: Docker Stats
run: docker stats --no-stream

integration-test-patient-purge:
needs: build
runs-on: ubuntu-24.04

steps:
- name: Check out Git repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4

- name: Install Blazectl
run: .github/scripts/install-blazectl.sh

- name: Download Blaze Image
uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4
with:
name: blaze-image
path: /tmp

- name: Load Blaze Image
run: docker load --input /tmp/blaze.tar

- name: Run Blaze
run: docker run --name blaze -d -e JAVA_TOOL_OPTIONS=-Xmx2g -e DB_BLOCK_CACHE_SIZE=2048 -e ENABLE_OPERATION_PATIENT_PURGE=true -p 8080:8080 --read-only --tmpfs /tmp:exec -v blaze-data:/app/data blaze:latest

- name: Wait for Blaze
run: .github/scripts/wait-for-url.sh http://localhost:8080/health

- name: Docker Logs
run: docker logs blaze

- name: Check Capability Statement
run: .github/scripts/check-capability-statement.sh

- name: Check Referential Integrity Enforced
run: .github/scripts/check-referential-integrity-enforced.sh

- name: Download Synthea Test Data
run: wget https://speicherwolke.uni-leipzig.de/index.php/s/kDsa2ifeMFdqK35/download/synthea-1000.tar

- name: Create Synthea Test Data Dir
run: mkdir test-data-synthea-1000

- name: Unpack Synthea Test Data
run: tar -C test-data-synthea-1000 -xf synthea-1000.tar

- name: Load Data
run: blazectl --no-progress --server http://localhost:8080/fhir upload test-data-synthea-1000

- name: Check Total-Number of Resources are 1099594
run: .github/scripts/check-total-number-of-resources.sh 1099594

- name: Patient Purge all
run: .github/scripts/patient-purge-all.sh

- name: Download All Resources
run: .github/scripts/download-all-resources.sh

- name: Download Patient Resources
run: .github/scripts/download-resources.sh Patient

- name: Check Resource Totals
run: .github/scripts/check-resource-totals-after-patient-purge.sh

- name: Docker Stats
run: docker stats --no-stream

not-enforcing-referential-integrity-test:
needs: build
runs-on: ubuntu-24.04
Expand Down
10 changes: 8 additions & 2 deletions docs/implementation/database.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Another data model is the document model used in document stores like [MongoDB][

There are two different sets of indices, ones which depend on the database value point in time `t` and the ones which only depend on the content hash of resource versions. The former indices are used to build up the database values were the latter are used to enable queries based on search parameters.

### Indices depending on t
### Transaction Indices

| Name | Key Parts | Value |
|-------------------|-----------|-------------------------------------------|
Expand Down Expand Up @@ -118,7 +118,7 @@ The `TypeStats` index keeps track of the total number of resources, and the numb

The `SystemStats` index keeps track of the total number of resources, and the number of changes to all resources at a particular point in time `t`. This statistic is used to populate the total count property in system listings and system history listings in case there are no filters applied.

### Indices not depending on t
### Search Param Indices

The indices not depending on `t` directly point to the resource versions by their content hash.

Expand Down Expand Up @@ -328,6 +328,12 @@ The `delete-history` command is used to delete the history of a resource.
| type | yes | string | resource type |
| id | yes | string | resource id |

#### Execution

* get all instance history entries
* add the `t` of the transaction as `purged-at?` to the value of each of the history entries not only in the ResourceAsOf index, but also in the TypeAsOf and SystemAsOf index
* remove the number of history entries purged from the number of changes of `type` and thw whole system

### Patient Purge

The `patient-purge` command is used to remove all current and historical versions for all resources in a patient compartment.
Expand Down
3 changes: 3 additions & 0 deletions modules/db/src/blaze/db/api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,9 @@
"Returns a CompletableFuture that will complete with the resource of
`resource-handle` or an anomaly in case of errors.
Note: If an deleted resource is pulled, a stub with type, id and meta will be
returned.
Functions applied after the returned future are executed on the common
ForkJoinPool."
[node-or-db resource-handle]
Expand Down
26 changes: 14 additions & 12 deletions modules/db/src/blaze/db/node/tx_indexer/verify.clj
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,21 @@
refs)))

(def ^:private inc-0 (fnil inc 0))
(def ^:private dec-0 (fnil dec 0))
(def ^:private minus-0 (fnil - 0))

(defn- inc-num-changes-and-total [stat]
(-> (update stat :num-changes inc-0)
(update :total inc-0)))

(defmethod verify "create" [db-before t res {:keys [type id hash refs]}]
(log/trace (verify-tx-cmd-create-msg type id))
(with-open [_ (prom/timer tx-u/duration-seconds "verify-create")]
(check-id-collision! db-before type id)
(let [tid (codec/tid type)]
(-> (update res :entries into (index-entries tid id t hash 1 :create refs))
(update :new-resources conj [type id])
(update-in [:stats tid :num-changes] inc-0)
(update-in [:stats tid :total] inc-0)))))
(update-in [:stats tid] inc-num-changes-and-total)))))

(defn- print-etags [ts]
(str/join "," (map (partial format "W/\"%d\"") ts)))
Expand Down Expand Up @@ -145,6 +149,7 @@
(and (some? old-t) (= if-none-match old-t))
(throw-anom (precondition-version-failed-anomaly type id if-none-match))

;; identical update, we will do nothing
(= old-hash hash)
res

Expand Down Expand Up @@ -203,7 +208,7 @@
(update :del-resources conj [type id])
(update-in [:stats tid :num-changes] inc-0))
op
(update-in [:stats tid :total] (fnil dec 0)))))))
(update-in [:stats tid :total] dec-0))))))

(def ^:private ^:const ^long delete-history-max 100000)

Expand All @@ -219,12 +224,9 @@
(defn- purge-entry [tid id t rh]
(rts/index-entries tid id (rh/t rh) (rh/hash rh) (rh/num-changes rh) (rh/op rh) t))

(defn- purge-entries [tid id t instance-history]
(into [] (mapcat (partial purge-entry tid id t)) instance-history))

(defn- add-delete-history-entries [entries tid id t instance-history]
(-> (update entries :entries into (purge-entries tid (codec/id-byte-string id) t instance-history))
(update-in [:stats tid :num-changes] minus-0 (count instance-history))))
(defn- add-purge-entries [res tid id t resource-handles]
(-> (update res :entries into (mapcat (partial purge-entry tid (codec/id-byte-string id) t)) resource-handles)
(update-in [:stats tid :num-changes] minus-0 (count resource-handles))))

(defmethod verify "delete-history"
[db-before t res {:keys [type id]}]
Expand All @@ -240,7 +242,7 @@
(throw-anom (too-many-history-entries-anom type id))

:else
(add-delete-history-entries res tid id t instance-history)))))
(add-purge-entries res tid id t instance-history)))))

;; like delete-history but also purges the current version
(defmethod verify "purge"
Expand All @@ -257,10 +259,10 @@
(throw-anom (too-many-history-entries-anom type id))

:else
(cond-> (add-delete-history-entries res tid id t instance-history)
(cond-> (add-purge-entries res tid id t instance-history)
(not (identical? :delete (rh/op (first instance-history))))
(-> (update :del-resources conj [type id])
(update-in [:stats tid :total] (fnil dec 0))))))))
(update-in [:stats tid :total] dec-0)))))))

(defmethod verify :default
[_db-before _t res _tx-cmd]
Expand Down
Loading

0 comments on commit 5c61491

Please sign in to comment.