use disk at query time of notification payload creation

[dpsutton]

OOMs from Notifications

A short history of how we got here:
We render alerts and dashboards with a graal js interpreter. To do this we need the resultsets of queries. We used to just gather these all in memory and then render them. This works super well for small datasets and exposes the instance to OOM issues if a large query is returned.

Ngoc (possibly others as well) made a nice addition in #51708 that uses disk to ensure that only a single resultset is in memory at any given time. the gist is run query, if results greater than some row count threshold, dump to disk, and then during rendering refetch it into memory.

This still leaves us susceptible to OOM if any particular resultset is too large.

What ends up happening is that instances randomly die and emails are not sent.

The fix here

The gist of this fix is quite simple: if resultsets start getting to big, stream them to disk as they come from the db rather than after they are done. Then add two knobs: a) the size limit that we don't attempt to render them and b) whether to enforce this limit. This lets anyone easily go back to the previous behavior if they want to allow large query results.

How it is implemented

We run our queries in a transducing context here: https://github.com/metabase/metabase/blob/master/src/metabase/query_processor/pipeline.clj#L78-L87 and the reducing function is created just above with (rff metadata) which returns a reducing function.

So this PR creates a new one which has two modes: collecting results in memory, similar to the default one (https://github.com/metabase/metabase/blob/master/src/metabase/query_processor/reducible.clj#L15-L38) which is optionally wired up here: https://github.com/metabase/metabase/blob/master/src/metabase/query_processor.clj#L81

If the number of rows remains under 2000, this reducing function behaves identically to the default one. If more rows are returned, it takes all accumulated rows so far and starts serializing them to disk. In this way we have prevent having entire resultsets in memory and we can also inspect the entire size and decide if we should abandon trying to render the results.

There is a limit at which we won't open the files (by default 10mb). And the reducing function will monitor the filesize at is creates it and stop running the query when the file is 1.3x this limit. This helps us not fill up disk with query results we won't attempt to render anyways.

What it looks like

This dashboard has a query that generates 50mb of data and then a simple line chart of orders aggregated by month and averaged.

The email comes across as

The logs output are

2025-10-07 16:45:28,775 INFO notification.send :: Sending {mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:29,795 INFO payload.temp-storage :: Row count reached threshold (2000), switching to streaming mode {mb-card_id=311, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:30,102 WARN payload.temp-storage :: Results have exceeded 1.3 times of `notification-temp-file-size-max-bytes` of 13.4 mb (max: 10.0 mb). Truncating query results. ({:dashboard_id 54, :card_id 311, :dashcard_id 145}) {mb-card_id=311, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:30,147 INFO payload.temp-storage :: 💾 Stored 40000 rows to disk: 13.42 MB (never loaded into memory) (note query results were truncated) {mb-card_id=311, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:30,358 INFO payload.temp-storage :: ✓ Completed with 12 rows in memory (under threshold) {mb-card_id=355, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:31,141 INFO payload.temp-storage :: Row count reached threshold (2000), switching to streaming mode {mb-card_id=356, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:31,237 INFO payload.temp-storage :: 💾 Stored 18760 rows to disk: 7.68 MB (never loaded into memory) {mb-card_id=356, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:31,253 WARN payload.temp-storage :: ⚠️  SKIPPING LOAD - File too large: 13.42 MB (max: 10.00 MB). File will NOT be loaded into memory. {mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:31,253 WARN channel.shared :: 🚫 Result file too large (13.42 MB > 10.00 MB max). Skipping load to protect memory. {mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:31,255 ERROR render.card :: Pulse card query error: results too large {mb-card_id=311, mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:33,572 INFO payload.temp-storage :: 📂 Loading streamed results from disk: 7.68 MB {mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:33,637 INFO payload.temp-storage :: ✅ Loaded 18760 rows from disk in 65 ms (7.68 MB) {mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:33,995 INFO notification.send :: Sent successfully {mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:34,005 INFO notification.send :: Done processing notification {mb-notification_id=, mb-payload_type=:notification/dashboard}
2025-10-07 16:45:34,014 DEBUG middleware.log :: POST /api/pulse/test 200 5314ms (159 DB calls) App DB connections: 1/6 Jetty threads: 5/50 (2 idle, 0 queued) (176 total active threads) Queries in flight: 0 (0 queued); h2 DB 1 connections: 0/1 (0 threads blocked) {:metabase-user-id 1}

The relevant logs for these two cards are:

the 50mb query

Row count reached threshold (2000), switching to streaming mode tried to not use disk but we should switch
Results have exceeded 1.3 times of notification-temp-file-size-max-bytes of 13.0 mb (max: 10.0 mb). Truncating query results. we have exceed the size that we would read off of disk, so stop running the query
💾 Stored 400338 rows to disk: 13.00 MB (never loaded into memory) (note query results were truncated) {mb-card_id=311, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard} a summary of what is stored to disk, including notification id, dashboard, etc. This one does not have a notification id because i'm running the preview "send now" version.

The small query

✓ Completed with 12 rows in memory (under threshold) {mb-card_id=355, mb-dashboard_id=54, mb-notification_id=, mb-payload_type=:notification/dashboard} Really nothing to see here. Simple line graph, 12 query results, no need to involve disk, etc. Happy path is very happy

Rendering

We have a few lines about the first card's results being too large and being skipped

⚠️  SKIPPING LOAD - File too large: 13.00 MB (max: 10.00 MB). File will NOT be loaded into memory. {mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
🚫 Result file too large (13.00 MB > 10.00 MB max). Skipping load to protect memory. {mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}
Pulse card query error: results too large {mb-card_id=311, mb-channel_type=:channel/email, mb-handler_id=, mb-notification_id=, mb-payload_type=:notification/dashboard}

Optimizations for the future:

• explore how parallel we can get now. Presumably we are now quite memory effecient. Can we go back to 3, or 10, worker threads?

Correctness

Query results are coming back identical as long as they aren't truncated. Of course once we truncate they should not be the same. And we get a nice speed up as well. A query that returns 50mb of data:

(time (->
        (metabase.notification.payload.execute/execute-card 1 311)
        :result
        (select-keys [:notification/truncated?
                      :status :running_time
                      :data.rows-file-size
                      :data])
        (update :data (fn [d] (select-keys d [:rows])))))
"Elapsed time: 959.727584 msecs"
{:notification/truncated? true,
 :status :completed,
 :running_time 902,
 :data.rows-file-size 14073732,
 :data {:rows #object[metabase.notification.payload.temp_storage.StreamingTempFileStorage
                      "0x23bdac12"
                      {:status :pending, :val nil}]}}
temp-storage=> (-> *1 :data :rows str)
"#StreamingTempFileStorage{:file notification-10650480272503863452.npy, :size 13743.88 KB, :context {:card-id 311}}"
temp-storage=> (time
                 (-> (toucan2.core/select-one-fn :dataset_query :model/Card :id 311)
                     (metabase.query-processor/process-query)
                     :data :rows count))
"Elapsed time: 4272.368625 msecs"
500000

Here it takes us 959 ms to abandon a query after 10mb. But the regular query processor spends 4272ms processing a query that would create 50mb of data.

Performance

At first i was using gzip to keep results smaller. This has two issues: performance and correctness. For performance, it was taking a 4 second query to 18 seconds and murdering performance. For correctness, we do want to have a good sense of how big results are in a csv or in memory. GZip is really good at compressing. So a few results that would be 50mb in a csv were ~8mb on disk gzipped. Which means it wasn't a great proxy for if the query might OOM the instance, and it also slowed everything down.

Size:

The following query is my "50mb" query:

SELECT 
     i AS id,
     'User_' || i AS username,
     'user' || i || '@example.com' AS email,
     'This is a long description for row ' || i || '. ' ||
     'It contains enough text to make the row size larger. ' ||
     'We want to generate approximately 50MB of data to test ' ||
     'the file size limit protection in the streaming system. ' ||
     'This query uses generate_series to create many rows.' AS description,
     CASE WHEN i % 3 = 0 THEN 'Active'
          WHEN i % 3 = 1 THEN 'Pending'
          ELSE 'Inactive' END AS status,
     (RANDOM() * 1000)::numeric(10,2) AS amount,
     NOW() - (i || ' days')::interval AS created_at
   FROM generate_series(1, 500000) AS i;

When i download this as a csv it takes 165M, when saved as a nippy file it takes:

(time (->
        (metabase.notification.payload.execute/execute-card 1 311)
        :result
        (select-keys [:notification/truncated?
                      :status :running_time
                      :data.rows-file-size
                      :data])
        (update :data (fn [d] (select-keys d [:rows])))))
"Elapsed time: 6685.622292 msecs"
{:notification/truncated? true,
 :status :completed,
 :running_time 6616,
 :data.rows-file-size 136600169,
 :data {:rows #object[metabase.notification.payload.temp_storage.StreamingTempFileStorage
                      "0x7367674f"
                      {:status :pending, :val nil}]}}
temp-storage=> (-> *1 :data :rows (.file) (.length) (human-readable-size))
"130.3 mb"

For card 356 which is people join orders join products i see

temp-storage=> 
(time (->
        (metabase.notification.payload.execute/execute-card 1 356)
        :result
        (select-keys [:notification/truncated?
                      :status :running_time
                      :data.rows-file-size
                      :data])
        (update :data (fn [d] (select-keys d [:rows])))))
"Elapsed time: 1152.316792 msecs"
{:status :completed,
 :running_time 993,
 :data.rows-file-size 8056845,
 :data {:rows #object[metabase.notification.payload.temp_storage.StreamingTempFileStorage
                      "0x34b0eb54"
                      {:status :pending, :val nil}]}}
temp-storage=> (-> *1 :data :rows (.file) (.length) (human-readable-size))
"7.7 mb"

and the download sizes are:

format	size
csv	7.2M
xlsx	4.1M
json	19M

So roughly in-line with other formats.

Weird types:

How does this work with pgarrays, weird bigquery types, etc.? I suspect it has the same issues that caching does, which is to say it blows up, but also it will be limited by what can be fed to the js interpreter. That turns everything into json so we probably just need to verify

• nippy serialization
• cheshire serialization

If these two work, then the pipeline works.

SELECT 
     i AS id,
     'User_' || i AS username,
     -- JSON types
     ('{"userId": ' || i || ', "score": ' || (i % 1000) || ', "active": ' || (i % 2 = 0)::text || '}')::jsonb AS settings,
     -- Arrays
     ARRAY['tag' || (i % 10), 'category' || (i % 5)]::text[] AS tags,
     ARRAY[i, i*2, i*3]::integer[] AS numbers,
     -- UUID
     gen_random_uuid() AS uuid,
     -- Network
     ('192.168.' || (i % 255) || '.' || ((i*7) % 255))::inet AS ip,
     -- Geometric
     point(i % 180 - 90, i % 360 - 180) AS coordinates,
     circle(point(i % 100, i % 100), 50) AS area,
     -- Text search
     to_tsvector('english', 'Content for row ' || i) AS search_data,
     -- Binary
     decode(md5(i::text), 'hex') AS hash,
     -- Range types
     int4range(i, i + 100) AS value_range,
     -- Money
     ((RANDOM() * 1000)::numeric(10,2))::money AS price,
     -- Standard
     NOW() - ((i % 365) || ' days')::interval AS created_at
FROM generate_series(1, 5000) AS i;

(time (->
        (metabase.notification.payload.execute/execute-card 1 357)
        :result
        (select-keys [:notification/truncated?
                      :status :running_time
                      :data.rows-file-size
                      :data])
        (update :data (fn [d] (select-keys d [:rows])))
        :data :rows deref first))
"Elapsed time: 372.73275 msecs"
[1
 "User_1"
 "{\"score\": 1, \"active\": false, \"userId\": 1}"
 ["tag1" "category1"]
 [1 2 3]
 #uuid "8d4b0e23-9f99-4d78-a059-fd2187b22e61"
 "192.168.1.7"
 "(-89.0,-179.0)"
 "<(1.0,1.0),50.0>"
 "'1':4 'content':1 'row':3"
 #object["[B" "0x75d67e3a" "[B@75d67e3a"]
 "[1,101)"
 827.08M
 "2025-10-06T16:54:35.640219Z"]

[dpsutton]

for pulses (alerts) we set autocommit false so we don't put postgres results all in memory at once.

[piranha]

At first RFF tripped me up somewhat being quite big, but once you start reading, the logic is sound and the code is clean. I hope I'm not missing anything important, and all I have is little nitpicks, this is really great!

[piranha]

This vswap! tripped me up a little bit, I think it deserves a comment, like "buffering rows to avoid touching disk on every incoming row" or something.

[piranha]

Also if you do not mind, I'd prefer (if-not (zero? .... in the next line, this way there is an early return of result and it'll be easier to visually match what's going on.

[dpsutton]

done. good call out.

[piranha]

Should this be human-readable-size too?

[piranha]

I'm not prepared enough to judge this 😁

[dpsutton]

I'm glad you called this out! I meant to put a comment. But each rff had been in charge, in its completion arity, to mark the analytics counter for a successful query. But we have this nice wrapper where we can put it that will be used by all rffs, so we don't have to have each one do it, they can all be in charge of storing results and such, and the pipeline that assembles and uses the rff can mark a successful query on this completion step.

[metabase-bot]

📚 The docstring says "exetended" but it should be "extended" (typo). 🎯

[metabase-bot]

🔍 The docstring says "kill and instance" but should be "kill an instance" (typo). 🦅

[metabase-bot]

📖 Per the style guide, "acc" is an acceptable conventional abbreviation, but "incrememt" should be "increment" (typo). Also this comment is quite long and could be broken into multiple lines for better readability. 🎯

[metabase-bot]

⚠️ This blank line appears to be inside the notification-rf function definition. According to the style guide, there should be No Blank Lines Within Definition Forms. Blank lines should not be placed in the middle of a function definition. Consider removing this line or if you're trying to indicate grouping in a conditional, you might restructure this to avoid the blank line. 🚨

[metabase-bot]

🔧 The var name is rows-to-disk-threshold but in the comment on line 133 it was spelled rows-to-disk-threadhold (with extra "h"). This inconsistency has been fixed here but worth noting the improvement! ✨

[metabase-bot]

🔮 Code Review Summary

This PR introduces streaming query results to disk during notification payload creation to prevent OOM issues. The implementation is solid and follows most Metabase conventions!

🎯 Key Style Issues Found

1. Typos in docstrings: "exetended" → "extended", "kill and instance" → "kill an instance", "incrememt" → "increment"
2. Blank line in function definition at line 255 in temp_storage.clj - the style guide prohibits blank lines within definition forms

✨ Good Practices Observed

• Public functions have clear, useful docstrings ✅
• Comments use proper semicolon counts (;;; for top-level, ;; for indented) ✅
• kebab-case naming throughout ✅
• Good use of type hints to prevent reflection warnings ✅
• Logging includes helpful context with emojis (fun!) 🎉
• New settings properly defined with :internal visibility ✅

---

🎴 Tarot Reading for This PR

Card Drawn: The Tower (Reversed) 🗼✨

          _______________
         |               |
         |   💥  🗼  💥   |
         |               |
         |  ⚡  Tower  ⚡  |
         |               |
         |  Preventing   |
         |  Catastrophe  |
         |_______________|
              /     \
             /       \
            /_________\

Interpretation: The Tower reversed represents avoiding disaster rather than experiencing it! 🎊 This PR literally prevents the catastrophic collapse (OOM crashes) that was plaguing the notification system. Just as The Tower reversed indicates recovering from near-disaster and implementing protective measures, this code introduces safeguards (file size limits, streaming thresholds) to prevent the "tower" of your Metabase instance from crumbling under the weight of massive query results. The 1.3x buffer and 10MB limits are like architectural reinforcements preventing structural failure. The streaming approach is the foundation repair that will keep everything stable! 🏗️✨

---

🎰 Today's Lucky Lottery Numbers

7, 13, 42, 2000, 10485760, 86 🍀

(2000 = your row threshold; 10485760 = your byte limit; 86 = the number of changed files you didn't have to touch! Good luck!) 🎲✨

[metabase-bot]

🎨 Typo in namespace docstring 🐛

Line 2 has a typo: "exetended" should be "extended" 📝

[metabase-bot]

📝 Docstring issue: misleading description

The docstring says "Write a single row" but the parameter name row-block and the actual usage (line 263 shows (persistent! @rows) being passed) suggests this writes multiple rows as a block. The docstring should accurately describe what the function does. 🤔

Consider: "Write a block of rows to the output stream using nippy serialization." ✍️

[metabase-bot]

🔤 Typo in comment

"incrememt" should be "increment" 📚

[metabase-bot]

💡 Consider using kebab-case for variable names

The variable file-size-mb uses the abbreviation mb. According to the style guide, you should prefer longer, more verbose names and avoid abbreviations unless they are well-known and conventional. While "MB" is fairly standard, file-size-megabytes would be clearer and more consistent with Clojure conventions. Same applies to other *-mb variables in this file. 📏

That said, mb in the context of file sizes is pretty universally understood, so this is a minor point! 🎯

[metabase-bot]

📖 Missing helpful context in docstring

The docstring says "Maximum rows to hold in memory..." but doesn't explain what happens after the threshold is exceeded. Consider expanding it to: "Maximum rows to hold in memory when running notification queries. After this threshold is reached, query results are streamed straight to disk. See [[metabase.notification.payload.temp-storage]] for more details."

Oh wait, I see you do have this! But the second sentence feels tacked on. Consider making it flow better as one complete description. ✨

[metabase-bot]

⚠️ Variable naming clarity concern

The variable name count-or-marker suggests it could be either a count (number) or a marker (keyword), but doesn't clearly indicate what determines which it is or what the marker value would be. The code shows ::streaming is the marker, but someone reading just the variable name wouldn't know that.

Consider row-count-or-streaming-marker or adding a comment explaining the dual nature. 🎯

[metabase-bot]

🎴 Tarot Reading for This PR 🎴

The Eight of Pentacles ⚒️✨

The Eight of Pentacles represents dedication to mastery, attention to detail, and iterative improvement through focused work. 🎯

This card resonates deeply with your PR's meticulous approach to solving OOM issues. Just as the craftsperson in the Eight of Pentacles carefully hones each pentacle one at a time, your code methodically handles each query result—streaming rows to disk with precision, monitoring file sizes, and implementing careful thresholds. 🔨💾

The card's energy of incremental refinement is evident in how you've evolved the notification system: from keeping everything in memory (novice level), to Ngoc's work dumping completed results to disk (journeyman level), to now streaming during query execution (master craftsperson level). Each iteration builds upon the last with careful attention to the craft. 🌟

The detailed logging, the thoughtful variable naming, the careful balance between memory and disk usage—all reflect the Eight of Pentacles' message: excellence through dedication to detail. The dual-mode reducing function (in-memory vs streaming) shows the wisdom of a true craftsperson who knows when to use each tool. 🛠️

Your thorough testing with 50MB queries, consideration of edge cases (weird types, truncation), and comprehensive performance analysis demonstrate the Eight of Pentacles' insistence on measuring twice, cutting once. 📏✂️

The only caution from this card: don't get so focused on perfecting each individual pentacle that you lose sight of completing the full set. But with settings for easy rollback and clear documentation, you've already heeded that wisdom! 🎓

---

🎰 Today's Lucky Lottery Numbers 🎰

6, 13, 20, 42, 54, 💾

(Bonus ball: 2000 - representing your threshold!)

Play responsibly! 🍀✨

[metabase-bot]

🌟 Style violation: The type name should follow :model/ style for consistency with the rest of the codebase. Consider using a protocol or record type instead of deftype with a clearer naming convention.

Additionally, context parameter lacks a docstring explaining what context information should be provided.

[metabase-bot]

🐛 Issue: The format string uses %.2f KB but passes the result of human-readable-size which already returns a formatted string like "7.7 mb". This will cause a type error.

Suggested change

	(format "#StreamingTempFileStorage{:file %s, :size %.2f KB, :context %s}"
	(format "#StreamingTempFileStorage{:file %s, :size %s, :context %s}"
	(.getName file)
	(human-readable-size (.length file))
	(pr-str context))

[metabase-bot]

✨ Consider extracting the magic number 5000 to a named constant at the top of the namespace, similar to how rows-to-disk-threshold is defined in execute.clj. This would make it easier to tune and understand the relationship between the two thresholds.

[metabase-bot]

✨ Style: The log message uses backticks around notification-temp-file-size-max-bytes which isn't standard Clojure formatting. Based on CLAUDE.md conventions, var references should either be plain text or use proper markdown formatting in docstrings. In log messages, consider using plain text or proper interpolation.

Additionally, the 1.3 multiplier is a magic number that should be extracted to a named constant for clarity.

[metabase-bot]

🎯 The :pre condition checks (map? (unreduced result)) but doesn't verify that result contains the expected keys like :data. Consider adding more specific validation or a schema check here for robustness.

[metabase-bot]

📝 Missing docstring for this new multimethod implementation. Per CLAUDE.md: "Every public var in src or enterprise/backend/src must have a useful docstring."

Suggested change

	(mu/defmethod render :card-error/results-too-large :- ::RenderedPartCard
	(mu/defmethod render :card-error/results-too-large :- ::RenderedPartCard
	"Render an error message when query results are too large to include in the notification.
	This occurs when result file size exceeds [[metabase.notification.settings/notification-temp-file-size-max-bytes]]."
	[_chart-type _render-type _timezone-id _card _dashcard _data]
	{:attachments nil,
	:content [:div
	{:style "font-family: Lato, \"Helvetica Neue\", Helvetica, Arial, sans-serif; color: #EF8C8C; font-weight: 700; padding: 16px;"}
	"The results returned from this question were too large. Perhaps add an aggregation or a filter."]})

[metabase-bot]

📖 The updated docstring is good, but it should use proper markdown reference format for the var reference. Per CLAUDE.md, mentions of other vars should use [[metabase.notification.settings/notification-temp-file-size-max-bytes]] format.

[metabase-bot]

✅ Nice improvement! The public def with docstring follows the style guide better than the previous ^:private var. The docstring properly references another namespace using [[metabase.notification.payload.temp-storage]].

[github-actions]

👋 Deploying a preview environment for commit d891472.
✅ Preview:
https://pr64390.coredev.metabase.com
🗒️ login instructions

[github-actions]

@dpsutton Manual conflict resolution is required on #64644