From 7eae9814efe613641a9b30542ba42e4e25bf1c02 Mon Sep 17 00:00:00 2001 From: Muki Kiboigo Date: Thu, 23 Jul 2026 11:38:39 -0700 Subject: [PATCH] rework get to be strictly deferred --- src/network/HttpClient.zig | 60 ++++++----- src/network/cache/Cache.zig | 16 ++- src/network/cache/SqliteCache.zig | 166 +++++++++++++++--------------- src/storage/sqlite/Sqlite.zig | 6 +- 4 files changed, 137 insertions(+), 111 deletions(-) diff --git a/src/network/HttpClient.zig b/src/network/HttpClient.zig index e68ba0838..49b463743 100644 --- a/src/network/HttpClient.zig +++ b/src/network/HttpClient.zig @@ -926,36 +926,48 @@ fn cacheLookup(self: *Client, transfer: *Transfer) !bool { var iter = req.headers.iterator(); const req_headers = try iter.collect(arena); - const cached = try cache.get(arena, .{ + const cache_result = cache.get(arena, .{ .url = req.url, .timestamp = std.Io.Clock.now(.real, lp.io).toSeconds(), .request_headers = req_headers.items, - }) orelse { - lp.metrics.http_cache.incr(.miss); - transfer._cache_intent = .store; - return false; + }) catch |e| blk: { + log.err(.cache, "failed to get", .{ .url = req.url, .err = e }); + break :blk .miss; }; - if (cached.expired == false) { - lp.metrics.http_cache.incr(.hit); - try transfer.bufferCached(cached); - return true; + switch (cache_result) { + .hit => |cached| { + lp.metrics.http_cache.incr(.hit); + try transfer.bufferCached(cached); + return true; + }, + .revalidate => |cached| { + log.debug(.cache, "revalidate cache entry", .{ + .url = req.url, + .etag = cached.metadata.etag, + .last_modified = cached.metadata.last_modified, + }); + if (cached.metadata.etag) |etag| { + try req.headers.add(try std.fmt.allocPrintSentinel(arena, "If-None-Match: {s}", .{etag}, 0)); + } + if (cached.metadata.last_modified) |lm| { + try req.headers.add(try std.fmt.allocPrintSentinel(arena, "If-Modified-Since: {s}", .{lm}, 0)); + } + transfer._cache_intent = .{ .revalidate = cached }; + return false; + }, + .miss => { + lp.metrics.http_cache.incr(.miss); + transfer._cache_intent = .store; + return false; + }, + .stale => { + lp.metrics.http_cache.incr(.miss); + cache.evict(req.url); + transfer._cache_intent = .store; + return false; + }, } - - // expired but with validators - log.debug(.cache, "revalidate with etag", .{ - .url = req.url, - .etag = cached.metadata.etag, - .last_modified = cached.metadata.last_modified, - }); - if (cached.metadata.etag) |etag| { - try req.headers.add(try std.fmt.allocPrintSentinel(arena, "If-None-Match: {s}", .{etag}, 0)); - } - if (cached.metadata.last_modified) |lm| { - try req.headers.add(try std.fmt.allocPrintSentinel(arena, "If-Modified-Since: {s}", .{lm}, 0)); - } - transfer._cache_intent = .{ .revalidate = cached }; - return false; } // 304 on a revalidation: renew the stored entry from the fresh headers and diff --git a/src/network/cache/Cache.zig b/src/network/cache/Cache.zig index e1685b90c..5f7472892 100644 --- a/src/network/cache/Cache.zig +++ b/src/network/cache/Cache.zig @@ -37,7 +37,7 @@ pub fn deinit(self: *Cache) void { }; } -pub fn get(self: *Cache, arena: std.mem.Allocator, req: CacheRequest) !?CachedResponse { +pub fn get(self: *Cache, arena: std.mem.Allocator, req: CacheRequest) !CacheGetResult { return switch (self.kind) { inline else => |*c| c.get(arena, req), }; @@ -232,13 +232,23 @@ pub const CachedData = union(enum) { } }; +pub const CacheGetResult = union(enum) { + // Fresh / Usable as is. + hit: CachedResponse, + /// Stale but has proper revalidators. Caller should make a conditional request and then + /// renew or put depending on Response. + revalidate: CachedResponse, + /// Cache Miss. + miss, + /// Stale entry with no revalidators. Must call `evict()` and should be treated as a miss. + stale, +}; + pub const CachedResponse = struct { metadata: CachedMetadata, data: CachedData, - expired: bool, pub fn format(self: *const CachedResponse, writer: *std.Io.Writer) !void { - try writer.print("expired={}, ", .{self.expired}); try writer.print("metadata=(", .{}); try self.metadata.format(writer); try writer.print("), data=", .{}); diff --git a/src/network/cache/SqliteCache.zig b/src/network/cache/SqliteCache.zig index 86b4b6c0b..cdfb5cb28 100644 --- a/src/network/cache/SqliteCache.zig +++ b/src/network/cache/SqliteCache.zig @@ -25,6 +25,7 @@ const log = lp.log; const CacheRequest = Cache.CacheRequest; const RenewResponse = Cache.RenewResponse; const CachedMetadata = Cache.CachedMetadata; +const CacheGetResult = Cache.CacheGetResult; const CachedResponse = Cache.CachedResponse; const CacheControl = Cache.CacheControl; const parseDeltaSeconds = Cache.parseDeltaSeconds; @@ -83,7 +84,7 @@ pub fn init(allocator: std.mem.Allocator, path: SqliteCachePath) !SqliteCache { log.err( .cache, "failed to make path", - .{ .kind = "httpCacheSqlitePath", .path = cache_dir, .err = e }, + .{ .kind = "SqliteCache", .path = cache_dir, .err = e }, ); return e; }; @@ -122,11 +123,11 @@ pub fn deinit(self: *SqliteCache) void { self.pool.deinit(self.allocator); } -pub fn get(self: *SqliteCache, arena: std.mem.Allocator, req: CacheRequest) !?CachedResponse { +pub fn get(self: *SqliteCache, arena: std.mem.Allocator, req: CacheRequest) !CacheGetResult { const conn = try self.pool.acquire(); defer self.pool.release(conn); - try conn.begin(.immediate); + try conn.begin(.deferred); defer conn.rollback() catch {}; var entry = try conn.row( @@ -135,7 +136,7 @@ pub fn get(self: *SqliteCache, arena: std.mem.Allocator, req: CacheRequest) !?Ca \\ from metadata where url = $1 , .{req.url}) orelse { log.debug(.cache, "miss", .{ .url = req.url, .reason = "missing" }); - return null; + return .miss; }; defer entry.deinit(); @@ -157,9 +158,7 @@ pub fn get(self: *SqliteCache, arena: std.mem.Allocator, req: CacheRequest) !?Ca // we are going to have to make a network request for this resource. if (expired and !has_validators) { log.debug(.cache, "miss", .{ .url = req.url, .reason = "expired with no validators" }); - try conn.exec("delete from metadata where url = $1", .{req.url}); - try conn.commit(); - return null; + return .stale; } var vary_headers: std.ArrayList(Http.Header) = .empty; @@ -186,7 +185,7 @@ pub fn get(self: *SqliteCache, arena: std.mem.Allocator, req: CacheRequest) !?Ca .expected = value, .got = incoming, }); - return null; + return .miss; } try vary_headers.append(arena, .{ @@ -232,20 +231,15 @@ pub fn get(self: *SqliteCache, arena: std.mem.Allocator, req: CacheRequest) !?Ca "select data from body where url = $1", .{req.url}, ) orelse { - log.debug(.cache, "miss", .{ .url = req.url, .reason = "missing body " }); - return null; + log.debug(.cache, "miss", .{ .url = req.url, .reason = "missing body" }); + return .stale; }; defer body_entry.deinit(); const body = try arena.dupe(u8, body_entry.get(Blob, 0).data); log.debug(.cache, "hit", .{ .url = req.url, .expired = expired }); - try conn.commit(); - - return .{ - .metadata = metadata, - .data = .{ .buffer = body }, - .expired = expired, - }; + const resp: CachedResponse = .{ .metadata = metadata, .data = .{ .buffer = body } }; + return if (expired) .{ .revalidate = resp } else .{ .hit = resp }; } pub fn put(self: *SqliteCache, meta: CachedMetadata, body: []const u8) !void { @@ -330,14 +324,6 @@ pub fn renew(self: *SqliteCache, _: std.mem.Allocator, req: RenewResponse) !void try conn.begin(.immediate); defer conn.rollback() catch {}; - const exists = try conn.row("select 1 from metadata where url = $1", .{req.url}); - if (exists) |*row| { - row.deinit(); - } else { - log.debug(.cache, "miss", .{ .url = req.url, .reason = "missing" }); - return error.CacheEntryNotFound; - } - var age_at_store: u64 = 0; var etag: ?[]const u8 = null; var last_modified: ?[]const u8 = null; @@ -374,6 +360,12 @@ pub fn renew(self: *SqliteCache, _: std.mem.Allocator, req: RenewResponse) !void req.url, }); + const affected = conn.changes(); + if (affected == 0) { + log.debug(.cache, "miss", .{ .url = req.url, .reason = "missing" }); + return error.CacheEntryNotFound; + } + var lower_name: [256]u8 = undefined; for (req.headers) |h| { if (h.name.len > lower_name.len) return error.HeaderNameTooLong; @@ -416,7 +408,7 @@ test "SqliteCache: basic put and get" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); - const now = std.Io.Timestamp.now(testing.io, .boot).toMilliseconds(); + const now = std.Io.Clock.now(.boot, lp.io).toSeconds(); const meta = CachedMetadata{ .url = "https://example.com", .content_type = "text/html", @@ -437,12 +429,12 @@ test "SqliteCache: basic put and get" { .timestamp = now, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; + ); - try testing.expectEqualStrings("hello world", result.data.buffer); - try testing.expectEqual(@as(u16, 200), result.metadata.status); - try testing.expectEqual(false, result.expired); - try testing.expectEqualStrings("text/html", result.metadata.content_type); + try testing.expect(result == .hit); + try testing.expectEqualStrings("hello world", result.hit.data.buffer); + try testing.expectEqual(@as(u16, 200), result.hit.metadata.status); + try testing.expectEqualStrings("text/html", result.hit.metadata.content_type); } test "SqliteCache: get expiration" { @@ -477,10 +469,9 @@ test "SqliteCache: get expiration" { .timestamp = now + 50, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqual(false, fresh.expired); + ); + try testing.expect(fresh == .hit); - // age = 200 + 900 = 1100 >= 1000: stale const stale = try cache.get( arena.allocator(), .{ @@ -488,8 +479,9 @@ test "SqliteCache: get expiration" { .timestamp = now + 200, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqual(true, stale.expired); + ); + try testing.expect(stale == .revalidate); + try testing.expectEqualStrings("hello world", stale.revalidate.data.buffer); } test "SqliteCache: get expiration (without validators)" { @@ -523,10 +515,9 @@ test "SqliteCache: get expiration (without validators)" { .timestamp = now + 50, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqual(false, fresh.expired); + ); + try testing.expect(fresh == .hit); - // age = 200 + 900 = 1100 >= 1000: stale const stale = try cache.get( arena.allocator(), .{ @@ -535,7 +526,7 @@ test "SqliteCache: get expiration (without validators)" { .request_headers = &.{}, }, ); - try testing.expectEqual(null, stale); + try testing.expect(stale == .stale); } test "SqliteCache: put override" { @@ -565,8 +556,9 @@ test "SqliteCache: put override" { .timestamp = 5000, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqualStrings("hello world", result.data.buffer); + ); + try testing.expect(result == .hit); + try testing.expectEqualStrings("hello world", result.hit.data.buffer); } { @@ -589,8 +581,9 @@ test "SqliteCache: put override" { .timestamp = 10000, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqualStrings("goodbye world", result.data.buffer); + ); + try testing.expect(result == .hit); + try testing.expectEqualStrings("goodbye world", result.hit.data.buffer); } } @@ -601,7 +594,7 @@ test "SqliteCache: vary hit and miss" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); - const now = std.Io.Timestamp.now(testing.io, .boot).toMilliseconds(); + const now = std.Io.Clock.now(.boot, testing.io).toSeconds(); const meta = CachedMetadata{ .url = "https://example.com", .content_type = "text/html", @@ -621,20 +614,23 @@ test "SqliteCache: vary hit and miss" { .url = "https://example.com", .timestamp = now, .request_headers = &.{.{ .name = "Accept-Encoding", .value = "gzip" }}, - }) orelse return error.CacheMiss; - try testing.expectEqualStrings("hello world", hit.data.buffer); + }); + try testing.expect(hit == .hit); + try testing.expectEqualStrings("hello world", hit.hit.data.buffer); - try testing.expectEqual(null, try cache.get(arena.allocator(), .{ + const mismatch = try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{.{ .name = "Accept-Encoding", .value = "br" }}, - })); + }); + try testing.expect(mismatch == .miss); - try testing.expectEqual(null, try cache.get(arena.allocator(), .{ + const missing_header = try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{}, - })); + }); + try testing.expect(missing_header == .miss); } test "SqliteCache: vary multiple headers" { @@ -644,7 +640,7 @@ test "SqliteCache: vary multiple headers" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); - const now = std.Io.Timestamp.now(testing.io, .boot).toMilliseconds(); + const now = std.Io.Clock.now(.boot, testing.io).toSeconds(); const meta = CachedMetadata{ .url = "https://example.com", .content_type = "text/html", @@ -668,17 +664,19 @@ test "SqliteCache: vary multiple headers" { .{ .name = "Accept-Encoding", .value = "gzip" }, .{ .name = "Accept-Language", .value = "en" }, }, - }) orelse return error.CacheMiss; - try testing.expectEqualStrings("hello world", hit.data.buffer); + }); + try testing.expect(hit == .hit); + try testing.expectEqualStrings("hello world", hit.hit.data.buffer); - try testing.expectEqual(null, try cache.get(arena.allocator(), .{ + const mismatch = try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{ .{ .name = "Accept-Encoding", .value = "gzip" }, .{ .name = "Accept-Language", .value = "fr" }, }, - })); + }); + try testing.expect(mismatch == .miss); } test "SqliteCache: clear removes all entries" { @@ -688,7 +686,7 @@ test "SqliteCache: clear removes all entries" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); - const now = std.Io.Timestamp.now(testing.io, .boot).toMilliseconds(); + const now = std.Io.Clock.now(.boot, testing.io).toSeconds(); try cache.put(.{ .url = "https://example.com/a", .content_type = "text/html", @@ -711,41 +709,41 @@ test "SqliteCache: clear removes all entries" { .vary_headers = &.{}, }, "body b"); - try testing.expect(null != try cache.get( + try testing.expect((try cache.get( arena.allocator(), .{ .url = "https://example.com/a", .timestamp = now, .request_headers = &.{}, }, - )); - try testing.expect(null != try cache.get( + )) == .hit); + try testing.expect((try cache.get( arena.allocator(), .{ .url = "https://example.com/b", .timestamp = now, .request_headers = &.{}, }, - )); + )) == .hit); try cache.clear(); - try testing.expectEqual(null, try cache.get( + try testing.expect((try cache.get( arena.allocator(), .{ .url = "https://example.com/a", .timestamp = now, .request_headers = &.{}, }, - )); - try testing.expectEqual(null, try cache.get( + )) == .miss); + try testing.expect((try cache.get( arena.allocator(), .{ .url = "https://example.com/b", .timestamp = now, .request_headers = &.{}, }, - )); + )) == .miss); } test "SqliteCache: put after clear works" { @@ -755,7 +753,7 @@ test "SqliteCache: put after clear works" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); - const now = std.Io.Timestamp.now(testing.io, .boot).toMilliseconds(); + const now = std.Io.Clock.now(.boot, testing.io).toSeconds(); const meta = CachedMetadata{ .url = "https://example.com", .content_type = "text/html", @@ -770,14 +768,14 @@ test "SqliteCache: put after clear works" { try cache.put(meta, "before clear"); try cache.clear(); - try testing.expectEqual(null, try cache.get( + try testing.expect((try cache.get( arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{}, }, - )); + )) == .miss); try cache.put(meta, "after clear"); const result = try cache.get( @@ -787,8 +785,9 @@ test "SqliteCache: put after clear works" { .timestamp = now, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqualStrings("after clear", result.data.buffer); + ); + try testing.expect(result == .hit); + try testing.expectEqualStrings("after clear", result.hit.data.buffer); } test "SqliteCache: evict removes entry" { @@ -798,7 +797,7 @@ test "SqliteCache: evict removes entry" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); - const now = std.Io.Timestamp.now(testing.io, .boot).toMilliseconds(); + const now = std.Io.Clock.now(.boot, testing.io).toSeconds(); const meta = CachedMetadata{ .url = "https://example.com", .content_type = "text/html", @@ -812,21 +811,22 @@ test "SqliteCache: evict removes entry" { try cache.put(meta, "hello world"); - _ = try cache.get( + const before = try cache.get( arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{} }, - ) orelse return error.CacheMiss; + ); + try testing.expect(before == .hit); cache.evict("https://example.com"); - try testing.expectEqual(null, try cache.get( + try testing.expect((try cache.get( arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{}, }, - )); + )) == .miss); } test "SqliteCache: renew refreshes expiry" { @@ -862,10 +862,9 @@ test "SqliteCache: renew refreshes expiry" { .timestamp = now + 1200, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqual(false, fresh.expired); + ); + try testing.expect(fresh == .hit); - // Expires at now+500+1000 = now+1500 const stale = try cache.get( arena.allocator(), .{ @@ -873,8 +872,8 @@ test "SqliteCache: renew refreshes expiry" { .timestamp = now + 1500, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqual(true, stale.expired); + ); + try testing.expect(stale == .revalidate); } test "SqliteCache: renew preserves body" { @@ -884,7 +883,7 @@ test "SqliteCache: renew preserves body" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); - const now = std.Io.Timestamp.now(testing.io, .boot).toMilliseconds(); + const now = std.Io.Clock.now(.boot, testing.io).toSeconds(); try cache.put(.{ .url = "https://example.com", .content_type = "text/html", @@ -908,6 +907,7 @@ test "SqliteCache: renew preserves body" { .timestamp = now + 100, .request_headers = &.{}, }, - ) orelse return error.CacheMiss; - try testing.expectEqualStrings("original body", result.data.buffer); + ); + try testing.expect(result == .hit); + try testing.expectEqualStrings("original body", result.hit.data.buffer); } diff --git a/src/storage/sqlite/Sqlite.zig b/src/storage/sqlite/Sqlite.zig index bb3c5bbb9..461a09f21 100644 --- a/src/storage/sqlite/Sqlite.zig +++ b/src/storage/sqlite/Sqlite.zig @@ -75,7 +75,7 @@ pub const Migrations = struct { } try conn.exec( "insert into migrations (id, applied_at) values ($1, $2)", - .{ @as(i64, @intCast(i + 1)), std.Io.Timestamp.now(lp.io, .boot).toMilliseconds() }, + .{ @as(i64, @intCast(i + 1)), std.Io.Clock.now(.boot, lp.io).toSeconds() }, ); } @@ -177,6 +177,10 @@ pub const Conn = struct { } } + pub fn changes(self: Conn) i64 { + return @intCast(c.sqlite3_changes(self.conn)); + } + pub fn commit(self: Conn) !void { try self.exec("commit", .{}); }