From 9e22f4467f0cadb1f800f2aacecaeccd244cb35d Mon Sep 17 00:00:00 2001 From: Martin Hutchinson Date: Mon, 9 Dec 2024 11:32:39 +0000 Subject: [PATCH] [MySQL] complete entry bundle implementation MySQL API now checks that the number of entries returned in a bundle is not smaller than the number requested. It does this by parsing the entry bundle. This could be optimized by storing the bundle size in the table. Now this is fixed, the conformance personality is updated to set cache headers on the entry bundles to allow aggressive caching. Also fixed the checkpoint implementation to never cache, and handle not found properly. --- cmd/conformance/mysql/main.go | 17 +++++++++-------- storage/mysql/mysql.go | 21 +++++++++++++++------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/cmd/conformance/mysql/main.go b/cmd/conformance/mysql/main.go index d8f7a4c6..552980bc 100644 --- a/cmd/conformance/mysql/main.go +++ b/cmd/conformance/mysql/main.go @@ -143,15 +143,19 @@ func configureTilesReadAPI(mux *http.ServeMux, storage *mysql.Storage) { mux.HandleFunc("GET /checkpoint", func(w http.ResponseWriter, r *http.Request) { checkpoint, err := storage.ReadCheckpoint(r.Context()) if err != nil { + if errors.Is(err, fs.ErrNotExist) { + w.WriteHeader(http.StatusNotFound) + return + } klog.Errorf("/checkpoint: %v", err) w.WriteHeader(http.StatusInternalServerError) return } - if checkpoint == nil { - w.WriteHeader(http.StatusNotFound) - return - } + // Don't cache checkpoints as the endpoint refreshes regularly. + // A personality that wanted to _could_ set a small cache time here which was no higher + // than the checkpoint publish interval. + w.Header().Set("Cache-Control", "no-cache") if _, err := w.Write(checkpoint); err != nil { klog.Errorf("/checkpoint: %v", err) return @@ -208,10 +212,7 @@ func configureTilesReadAPI(mux *http.ServeMux, storage *mysql.Storage) { return } - // TODO: Add immutable Cache-Control header. - // Only do this once we're sure we're returning the right number of entries - // Currently a user can request a full tile and we can return a partial tile. - // If cache headers were set then this could cause caches to be poisoned. + w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") if _, err := w.Write(entryBundle); err != nil { klog.Errorf("/tile/entries/{index...}: %v", err) diff --git a/storage/mysql/mysql.go b/storage/mysql/mysql.go index 8ac67862..df378bf9 100644 --- a/storage/mysql/mysql.go +++ b/storage/mysql/mysql.go @@ -293,12 +293,9 @@ func (s *Storage) writeTile(ctx context.Context, tx *sql.Tx, level, index uint64 // ReadEntryBundle returns the log entries at the given index. // If the entry bundle is not found, it returns os.ErrNotExist. // -// TODO: Handle the following scenarios: -// 1. Full tile request with full tile output: Return full tile. -// 2. Full tile request with partial tile output: Return error. -// 3. Partial tile request with full/larger partial tile output: Return trimmed partial tile with correct tile width. -// 4. Partial tile request with partial tile (same width) output: Return partial tile. -// 5. Partial tile request with smaller partial tile output: Return error. +// Note that if a partial tile is requested, but a larger tile is available, this +// will return the largest tile available. This could be trimmed to return only the +// number of entries specifically requested if this behaviour becomes problematic. func (s *Storage) ReadEntryBundle(ctx context.Context, index, treeSize uint64) ([]byte, error) { row := s.db.QueryRowContext(ctx, selectTiledLeavesSQL, index) if err := row.Err(); err != nil { @@ -313,6 +310,18 @@ func (s *Storage) ReadEntryBundle(ctx context.Context, index, treeSize uint64) ( return nil, fmt.Errorf("scan entry bundle: %v", err) } + // If the user has requested a size larger than we have, they can't have it + // Parsing the bundle here is fast, but faster would be to record the partial bundle + // size in the DB and just read the int from there. + requestedWidth := int(partialTileSize(0, index, treeSize)) + parsed := api.EntryBundle{} + if err := parsed.UnmarshalText(entryBundle); err != nil { + return nil, fmt.Errorf("failed to parse entry bundle: %v", err) + } + if requestedWidth > len(parsed.Entries) { + return nil, os.ErrNotExist + } + return entryBundle, nil }