Skip policy_membership writes when passes value is unchanged

[getvictor]

Problem

RecordPolicyQueryExecutions in server/datastore/mysql/policies.go runs on every osquery distributed-write that includes policy results. It UPSERTs every incoming (policy_id, host_id, passes) triple via INSERT INTO policy_membership ... ON DUPLICATE KEY UPDATE updated_at=VALUES(updated_at), passes=VALUES(passes), regardless of whether passes actually changed since the previous run. With N hosts and M policies-per-host, every check-in writes N×M rows even when steady-state passing/failing rates are stable.

The same function already does the read needed to skip the write. FlippingPoliciesForHost selects the previous (policy_id, passes) rows from policy_membership and computes both newFailing and newPassing — the exact set of rows that actually need a passes change. The current code only consumes newPassing (for retry-reset bookkeeping) and discards newFailing, so it falls back to UPSERTing everything.

Impact

This path's cost depends on the stability of each host's policy results. First-time submissions (right after enrollment, or after a policy is added or its query changes) genuinely need to write — there is no prior row to compare against. Steady-state submissions on a stable host population are mostly no-op UPSERTs: passes matches what's already in the row.

The 2026-04-25 loadtest on victor-484 captured both regimes. During the enrollment wave that scaled the fleet from 10,500 to 20,000 hosts (~19:55–20:10Z), INSERT INTO policy_membership ... ON DUPLICATE KEY UPDATE peaked at ~1,277 s of writer wall in a single 5-min bucket — about 4.3 s of writer wall per second of clock, dominating every other writer that interval. Once the wave settled, the same statement fell to ~17 s over a 15-min window and dropped to ~#13 in the writer top-15, well below ambient writes like scheduled_query_stats and host_seen_times.

The fix does not change the enrollment-burst cost — those inserts have to land. What it removes is the UPSERT cost for unchanged-passes rows, which is the dominant case in long-running operation: a Fleet that's been up long enough for hosts to have submitted at least one round of policy results, with stable pass/fail rates, sees most RecordPolicyQueryExecutions writes become no-ops without this change. The savings compound over time and reduce writer-side amplification when an enrollment wave or policy edit lands on top of ambient load.

Proposed fix

Use the result of FlippingPoliciesForHost to build the UPSERT batch only from newFailing ∪ newPassing instead of all incoming results. Rows whose previous passes value matches the incoming value are skipped — they're true no-ops.

Two specifics:

The current code calls FlippingPoliciesForHost for the retry-reset path but throws away newFailing. Capture both and use their union. In the SubmitDistributedQueryResults path that pre-computes newlyPassingPolicyIDs and passes it in (so the inner call is skipped), pass both newFailing and newPassing through to RecordPolicyQueryExecutions, or fall back to a fresh FlippingPoliciesForHost read locally — the read is small (single host, small policy count) and the write savings dominate.

updated_at semantics shift slightly: today it refreshes every check-in regardless of state change; after the fix it refreshes only when passes actually flips. This is arguably more useful (it now means "last time membership state changed"), but verify no consumer reads updated_at as a "last seen" timestamp. The existing policy_updated_at on the hosts table already serves the per-host last-policy-run signal and is unchanged by this fix.

Evidence

Loadtest writer top-ops (2026-04-25, victor-484): INSERT INTO policy_membership ... ON DUPLICATE KEY UPDATE peaked at ~1,277 s in a single 5-min bucket and ~931 s over a 15-min window during the 19:55–20:10Z enrollment wave (10,500 → 20,000 hosts). After the wave settled, the same statement fell to ~17 s over a 15-min window and dropped out of the writer top-15 entirely.

Existing flipping-detection that already does the work to identify writeable rows: FlippingPoliciesForHost in server/datastore/mysql/policies.go.

Current write site: RecordPolicyQueryExecutions in server/datastore/mysql/policies.go.

Related top-2 reader, also touching policy_membership: SELECT SUM(1 - pm.passes) AS n_failed FROM policy_membership pm WHERE pm.host_id = ? at ~107 s wall over the same window.

[getvictor]

FYI. @lucasmrod

[raju249]

@lucasmrod @getvictor - I found this one too. I think this is straightforward to follow, thanks for the approach in the description.

Can I take this one too? I can work on this and the other where we are collaborating together. wdyt?

[raju249]

@lucasmrod @getvictor - I grepped every reader of policy_membership in server/, cmd/, and ee/ (non-test). All readers — FlippingPoliciesForHost, FailingPoliciesCount, ListPoliciesForHost, IsPolicyFailing, updateHostIssuesFailingPolicies paths, ConditionalAccessBypassDevice, GetTeamHostsPolicyMemberships, and the policyMembershipJoin filter in ListHosts — touch only passes, host_id, and policy_id. No in-tree reader of policy_membership.updated_at.

The exposed types HostPolicyMembershipData and PolicyMembershipResult don't include the column either, so it's invisible at the API layer.

The only caveat is if some other consumer is depending on the value of updated_at for their own workflows, they may get impacted.

I will move ahead with the approach and here and we can discuss further on the PR. Does that make sense?

[raju249]

Found a interesting point while working through the implementation of this issue.

Working through newFailing (Union) newPassing as the UPSERT batch, I noticed FlippingPoliciesForHost doesn't track first-run-passing policies — they aren't transitions, so for the automation paths that's correct, but it means under the proposed fix those rows would never get written. Subsequent unchanged reports would also skip them, so policy_membership would never gain a row for any host that's been passing a policy from day one.

The downstream effect is UI-side: the host-details page and the "passing this policy" host filter both depend on those rows existing. They'd silently show empty status / miss hosts whose pass-history is entirely first-run-passing. The failure-counting paths don't care, so those are unaffected.

Three ways forward:

1. Treat first-run-passing as newPassing. Smallest change, but webhooks would now start firing on first run (might be wanted, might not).
2. Add a firstRunPassing return alongside the flips. Keeps webhook semantics intact.
3. Inside RecordPolicyQueryExecutions, do a small extra read to identify rows where the stored value differs from incoming or no row exists. Leaves FlippingPoliciesForHost alone.

Leaning (3) — keeps webhook behavior the same and uses the kind of small read you flagged as acceptable. Happy to go (2) instead if you'd rather not duplicate the read shape. Let me know which way to take this.

@lucasmrod @getvictor

[getvictor]

@raju249 I don't quite follow the last issue and I don't have time to dig in right now. This issue is a performance optimization, so everything should work as it did before, and the only difference would be that the updated_at stays at the last time when the policy changed state.