Skip to content

Commit

Permalink
[MySQL] complete entry bundle implementation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mhutchinson committed Dec 9, 2024
1 parent 6106156 commit ee2408d
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
17 changes: 9 additions & 8 deletions cmd/conformance/mysql/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 15 additions & 6 deletions storage/mysql/mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down

0 comments on commit ee2408d

Please sign in to comment.