Fresh Forever

My agent caches every Amazon product it reads, in Postgres, with a TTL and a price history. The cache is not only about speed:

@moduledoc """
Product cache and price history.

The cache protects as much as it optimises: never asking Amazon twice for the same
ASIN directly reduces exposure to blocking.
"""

Twenty lines of write path, and I got it wrong three times in a row, where each fix is what made the next bug possible. The third is the one worth writing down: two behaviours that are each correct, and that only became a bug once they sat next to each other.

Twenty products, and nobody to write them down

The first symptom was a page that stayed empty. My agent has a small LiveView showing what it has recently observed, with price sparklines, and a successful search left it blank.

Only the two single-product tools, amazon_get_price and amazon_get_product_details, ever wrote to the cache. I had wired the cache onto the tools that read one product, and never asked what the tools reading twenty were supposed to do with what they saw.

That sounds like a one-line fix. A search hit has an ASIN, a title, a URL, a price most of the time. Write each hit, done.

A search hit is not a product

Writing them with the existing store/3 would have been wrong, and this is where the actual subject of this post starts.

store/3 is a full overwrite, which is exactly right for the tool it was written for: both product-detail tools parse the whole page and hand over a complete product. A search hit is a different animal. It has no brand, no seller, no feature bullets, because the results page never showed them.

Overwriting is therefore a data loss with extra steps. Search a product you had already fetched in detail, and the rich entry becomes a thin one. Same ASIN, same key, less knowledge than before, and the LiveView I was trying to fill would now show less than it did.

So search hits go through a second write path that merges instead of replacing, built on Postgres’s jsonb concatenation:

merge_query =
  from product in Product,
    update: [
      set: [
        payload: fragment("? || ?", product.payload, type(^payload, :map)),
        fetched_at: ^fetched_at_on_merge,
        updated_at: ^now
      ]
    ]

product =
  %Product{}
  |> Ecto.Changeset.change(asin: asin, domain: domain, payload: payload, fetched_at: now)
  |> Repo.insert!(
    on_conflict: merge_query,
    conflict_target: [:asin, :domain],
    returning: true
  )

Inside an ON CONFLICT DO UPDATE, the query’s binding refers to the row already in the table, so product.payload on the left of the || is the existing payload and the parameter on the right is the incoming one. Right wins on conflict, everything else survives. One statement, no read followed by a write, nothing to race against.

Two facts a jsonb || cannot tell apart

The merge was still wrong, and it was wrong on nearly every real search.

|| does not distinguish a key that is absent from a key that is present with a null value. A present null on the right wins over a real value on the left, precisely as any absent key would lose. Both shapes look the same to it, and they are not the same fact at all. One says “I have nothing to say about the price”. The other says “the price is nothing”.

My search parser only ever produces the second one. It builds every hit with the same keys, filling "price", "currency", "rating", "reviews_count" and "image_url" with nil when it could not parse them, never omitting them. That is a perfectly reasonable thing for a parser to do, and it meant that every search hit whose price could not be read erased the price of a richer entry cached for the same ASIN. The bug I had just avoided with the merge, reintroduced through the door the merge left open.

The fix is one line, and where it lives matters more than what it does:

def merge(asin, domain, payload) do
  now = DateTime.utc_now()
  payload = drop_nil_values(payload)

Not in the search tool that had the problem. In merge/3, before anything else, so no future caller can reintroduce this with nils of its own. A rule that lives at the call site is a rule that holds until somebody writes a second call site.

The test that agreed with me instead of testing me

Here is the part I keep thinking about.

I had already written a test called “merging preserves an existing key the new payload does not carry”. It passed. It kept passing while the bug was live, and it was not a bad test, it just answered a question I had not asked carefully enough:

{:ok, _first} =
  Catalog.store(@asin, @domain, %{
    "asin" => @asin,
    "title" => "Un livre",
    "brand" => "Une maison d'édition",
    "seller" => "Un vendeur"
  })

{:ok, _second} = Catalog.merge(@asin, @domain, %{"asin" => @asin, "price" => 999})

assert {:ok, payload} = Catalog.fetch_fresh(@asin, @domain)
assert payload["brand"] == "Une maison d'édition"
assert payload["seller"] == "Un vendeur"

The keys under test are brand and seller, and they are absent from the incoming payload. So the test proved that an absent key survives a merge, which was true before the bug, during it, and after the fix. Nothing in the file exercised a key present with an explicit nil, and that is the only shape my search parser can produce. It always emits the key. The test covered a payload shape my own system never builds, and passing told me nothing about the one it builds on every single search.

So the regression test is now written in the shape reality actually has:

{:ok, _second} =
  Catalog.merge(@asin, @domain, %{
    "asin" => @asin,
    "price" => nil,
    "currency" => nil,
    "image_url" => nil
  })

assert {:ok, payload} = Catalog.fetch_fresh(@asin, @domain)
assert payload["price"] == 999

When a test for “the value survives” passes, it is worth checking which of the two shapes it survived. Only one of them was ever going to show up.

The stamp that renewed itself forever

Now the good one, and the reason this post exists.

merge/3 bumped fetched_at to now on every merge, which is what a write does. It also deliberately kept the existing price when the incoming payload had none, which is what the previous fix was for. Each behaviour is correct in isolation. I wrote them a few minutes apart and never looked at them together.

Together they say: a product I search regularly gets its freshness stamp renewed forever, without its price ever being re-read.

And fetched_at is what the TTL is checked against:

def fetch_fresh(asin, domain) do
  cutoff = DateTime.add(DateTime.utc_now(), -Config.cache_ttl_minutes() * 60, :second)

  query =
    from product in Product,
      where:
        product.asin == ^asin and product.domain == ^domain and product.fetched_at > ^cutoff

So the cache expiry had silently switched itself off, and not for a random subset. For exactly the products I looked at most often, which are the ones whose prices I actually wanted to be current. A cache that never expires the entries you use is not a cache with a bug in it, it is a stale answer machine with a TTL painted on the side.

There is a second cost, and I think it is the worse one. fetched_at is handed to the model as evidence of how current a result is. A price check that answers “observed two minutes ago” when the number was read three weeks ago has stopped being a slow cache and become a false statement about the world, made to the one reader who cannot check it. It is the same shape as the tool descriptions in last week’s post, reached from a completely different direction.

What a timestamp is allowed to mean

The fix required deciding what the column means, and then making every write path obey that single sentence. fetched_at means “when this data was actually observed”.

On an insert it is always now: a search hit is a thin observation, but it is a genuine first observation of that ASIN. On a merge into an existing row it advances only when the incoming payload actually carries a fresh price, the same is_integer/1 test that decides whether a price point gets recorded at all. Otherwise the row keeps the stamp it had. store/3 is untouched: a full product-page fetch has genuinely observed everything, so it sets the stamp unconditionally.

In Ecto that conditional lives in a dynamic:

fetched_at_on_merge =
  if fresh_price?(payload), do: dynamic(^now), else: dynamic([product], product.fetched_at)

Two branches, and the difference between them is not a value but a reference: either a literal timestamp, or the column’s own current value. It is the same trick as "payload" = c0."payload" || $7, applied to a column where I want the update to be a no-op.

I like this because of what the database receives. Same function, one insert! call, and two genuinely different statements. With a fresh price:

INSERT INTO "catalog_products" AS c0 (...) VALUES ($1,$2,$3,$4,$5,$6)
ON CONFLICT ("asin","domain") DO UPDATE
SET "payload" = c0."payload" || $7::jsonb, "fetched_at" = $8, "updated_at" = $9

Without one:

INSERT INTO "catalog_products" AS c0 (...) VALUES ($1,$2,$3,$4,$5,$6)
ON CONFLICT ("asin","domain") DO UPDATE
SET "payload" = c0."payload" || $7::jsonb, "fetched_at" = c0."fetched_at", "updated_at" = $8

"fetched_at" = c0."fetched_at" is the whole fix, and it is still one round trip, still atomic, with no branch in my code that reads the row first to decide what to write. Both statements come from the query log of an actual merge rather than being reconstructed from the Elixir, with the column list and the RETURNING clause elided for width.

What a write knows

All three bugs come from one question I never asked while writing the code: what does this write actually know?

A thin write knows less than the row it lands on, so it must not be allowed to replace it, which is the entire difference between store/3 and merge/3. It also has to say which of its empty fields are genuinely empty and which it simply never saw, because jsonb || cannot tell those apart and lets a nil win by default. That decision belongs inside the write path rather than at the call site, where the next caller would have to remember it.

The timestamp is the one I would still get wrong today if I were not watching for it. A stamp is a claim about an event, and “update the timestamp on write” is such an ordinary reflex that it never feels like a claim at all. That is why this one went unnoticed for so long: nothing in that code looked like a bug, because separately, none of it was one.

The seven tests that hold this down read like the specification the three bugs turned out to be:

merging into an empty cache stores the given payload
merging preserves an existing key the new payload does not carry
merging overwrites a key the new payload does carry
merging records a price point when the new payload carries a price
merging a payload where a key is present but explicitly nil does not erase the existing value
merging a payload with no fresh price does not renew a stale entry's freshness
merging a payload that carries a fresh price renews freshness