Skip to content

Commit c67655a

Browse files
MaineK00nclaude
andcommitted
fix(server): own the vuls2 db fetch for the process instead of per request
Server mode created a vuls2 db session per request, so every request decided for itself whether the db was due for a download and fetched it. A pod that came up with no db on disk therefore had each arriving request start its own multi-gigabyte fetch into the same directory, several at a time, each slow enough to trip the registry's stream limits and start over. Hoist the db's lifecycle out of the request path: - SharedDB downloads the db and keeps it current in the background, and hands each request a session with SkipUpdate forced on. Only the goroutine that prepares and refreshes fetches, so a fetch is single-flight and never blocks a request, and a request can never become another thing that pulls a db. /health reports 503 until there is a db to serve and /vuls refuses rather than fetching one of its own. Point a readiness probe at /health: a liveness probe would restart the process partway through the first fetch and start it over. - Each request opens its own handle and carries its own read cache. Sharing one open handle was tried and reverted: opening the db is an mmap of a file the page cache already holds, ~50us on a full-size ~11 GB db, while a session shared across requests cannot carry vuls2's read cache, since that cache never evicts and one outliving a request would grow until the process is OOM-killed. Going without it makes enrichment re-read and re-unmarshal the same advisory and vulnerability records once per root that references them: enriching a 4873-CVE result measured 2.4s with a cache and 150s without. - Startup adopts whatever usable db is already on disk before it considers downloading one, so a process that has a db serves right away rather than after a full fetch. A refresh that fails leaves the working db in place. - A refresh resolves the repository manifest and skips the download when its digest matches the one recorded in the local db. Going by timestamps alone, a nightly db past the staleness window looks due on every check for as long as it lives, which re-fetched gigabytes hourly even when the tag had not moved. - Record the db digest on the global config from SharedDB alone rather than from every session open. Concurrent requests wrote it while detector.DetectPkgCves read the same global to stamp the result, which was a data race. - Size detection workers by GOMAXPROCS rather than NumCPU, which reports the machine's CPU count even when a cgroup quota lets far fewer of them run. - Bound concurrent detections with -max-concurrency (default GOMAXPROCS). One detection holds every CVE it finds plus a read cache that measured 0.4-0.8 GB on heavy servers, so an unbounded number of them oversubscribes memory badly enough to stall the server. Requests queue on it rather than being rejected, and a slot is released only after the session it admitted has been closed. Refs #2613 Refs #2615 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 86d476c commit c67655a

9 files changed

Lines changed: 854 additions & 50 deletions

File tree

detector/vuls2/db.go

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package vuls2
22

33
import (
4+
"context"
45
"os"
56
"path/filepath"
67
"time"
78

89
"github.com/pkg/errors"
910
bolt "go.etcd.io/bbolt"
1011
"golang.org/x/xerrors"
12+
"oras.land/oras-go/v2/registry/remote"
1113

1214
"github.com/future-architect/vuls/config"
1315
"github.com/future-architect/vuls/logging"
@@ -24,7 +26,18 @@ var (
2426
}()
2527
)
2628

27-
func newDBConfig(vuls2Conf config.Vuls2Conf, noProgress bool) (*session.Config, error) {
29+
// newDBConfig downloads or refreshes the db if due, validates its schema
30+
// version, and returns the session config to open it with.
31+
//
32+
// withCache decides whether the returned session carries a read cache. Any
33+
// session that answers a detection wants one: without it, enrichment re-reads
34+
// and re-unmarshals the same advisory and vulnerability records once per root
35+
// that references them, ~60x slower on a 4873-CVE result. The cache is
36+
// unbounded (a sync.Map that never evicts), so it is only safe on a session
37+
// whose lifetime is bounded — one server's detection run, or one request. A
38+
// session meant to outlive those must pass false, which is why SharedDB opens
39+
// the db without one when it is only checking that the db is usable.
40+
func newDBConfig(vuls2Conf config.Vuls2Conf, noProgress, withCache bool) (*session.Config, error) {
2841
willDownload, err := shouldDownload(vuls2Conf, time.Now())
2942
if err != nil {
3043
return nil, xerrors.Errorf("Failed to check whether to download vuls2 db. err: %w", err)
@@ -70,10 +83,62 @@ func newDBConfig(vuls2Conf config.Vuls2Conf, noProgress bool) (*session.Config,
7083
Type: "boltdb",
7184
Path: vuls2Conf.Path,
7285
Options: session.StorageOptions{BoltDB: &bolt.Options{ReadOnly: true}},
73-
WithCache: true,
86+
WithCache: withCache,
7487
}, nil
7588
}
7689

90+
// hasNewerRemote reports whether the repository holds a db other than the one on
91+
// disk, comparing the manifest digest that fetch recorded in the local db's
92+
// metadata against the digest the repository's reference resolves to now.
93+
//
94+
// shouldDownload can only tell that the local db is old enough to be worth
95+
// checking: it goes by timestamps, and the nightly db's LastModified is the
96+
// night it was built, so a db past the staleness window looks due on every
97+
// check until something replaces it. Downloading on that alone re-fetches
98+
// gigabytes on every check even when the tag has not moved. Resolving the
99+
// manifest costs one request, so it is worth asking before spending a download.
100+
func hasNewerRemote(ctx context.Context, vuls2Conf config.Vuls2Conf) (bool, error) {
101+
sesh, err := (&session.Config{
102+
Type: "boltdb",
103+
Path: vuls2Conf.Path,
104+
Options: session.StorageOptions{BoltDB: &bolt.Options{ReadOnly: true}},
105+
}).New()
106+
if err != nil {
107+
return false, xerrors.Errorf("Failed to new vuls2 db connection. path: %s, err: %w", vuls2Conf.Path, err)
108+
}
109+
110+
if err := sesh.Storage().Open(); err != nil {
111+
return false, xerrors.Errorf("Failed to open vuls2 db. path: %s, err: %w", vuls2Conf.Path, err)
112+
}
113+
defer sesh.Storage().Close()
114+
115+
metadata, err := sesh.Storage().GetMetadata()
116+
if err != nil {
117+
return false, xerrors.Errorf("Failed to get vuls2 db metadata. path: %s, err: %w", vuls2Conf.Path, err)
118+
}
119+
if metadata == nil || metadata.Digest == nil {
120+
// A db that was built locally rather than fetched carries no digest, so
121+
// there is nothing to compare it by and the repository's db counts as
122+
// the newer one.
123+
return true, nil
124+
}
125+
126+
repo, err := remote.NewRepository(vuls2Conf.Repository)
127+
if err != nil {
128+
return false, xerrors.Errorf("Failed to create client for %s. err: %w", vuls2Conf.Repository, err)
129+
}
130+
if repo.Reference.Reference == "" {
131+
return false, xerrors.Errorf("unexpected repository format. expected: %q, actual: %q", []string{"<repository>@<digest>", "<repository>:<tag>", "<repository>:<tag>@<digest>"}, vuls2Conf.Repository)
132+
}
133+
134+
desc, err := repo.Resolve(ctx, repo.Reference.Reference)
135+
if err != nil {
136+
return false, xerrors.Errorf("Failed to resolve %s. err: %w", vuls2Conf.Repository, err)
137+
}
138+
139+
return desc.Digest.String() != *metadata.Digest, nil
140+
}
141+
77142
func shouldDownload(vuls2Conf config.Vuls2Conf, now time.Time) (bool, error) {
78143
if _, err := os.Stat(vuls2Conf.Path); err != nil {
79144
if errors.Is(err, os.ErrNotExist) {

detector/vuls2/db_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package vuls2_test
22

33
import (
4+
"context"
45
"path/filepath"
56
"reflect"
67
"testing"
@@ -184,3 +185,62 @@ func schemaVersionBoltDB(t *testing.T) uint {
184185
}
185186
return sv
186187
}
188+
189+
func Test_hasNewerRemote(t *testing.T) {
190+
tests := []struct {
191+
name string
192+
digest *string
193+
writeDB bool
194+
repository string
195+
want bool
196+
wantErr bool
197+
}{
198+
{
199+
// A db that was built locally rather than fetched has no digest to
200+
// compare, so the repository's db counts as newer and this answers
201+
// without reaching the registry.
202+
name: "no digest recorded",
203+
writeDB: true,
204+
repository: "ghcr.io/vulsio/vuls-nightly-db:nightly",
205+
want: true,
206+
},
207+
{
208+
name: "no db file",
209+
repository: "ghcr.io/vulsio/vuls-nightly-db:nightly",
210+
wantErr: true,
211+
},
212+
{
213+
name: "unparsable repository",
214+
digest: func() *string { s := "sha256:a"; return &s }(),
215+
writeDB: true,
216+
repository: "not a repository",
217+
wantErr: true,
218+
},
219+
}
220+
for _, tt := range tests {
221+
t.Run(tt.name, func(t *testing.T) {
222+
conf := config.Vuls2Conf{
223+
Path: filepath.Join(t.TempDir(), "vuls.db"),
224+
Repository: tt.repository,
225+
}
226+
if tt.writeDB {
227+
if err := putMetadata(types.Metadata{
228+
LastModified: *parse("2024-01-02T00:00:00Z"),
229+
Downloaded: parse("2024-01-02T00:00:00Z"),
230+
SchemaVersion: schemaVersionBoltDB(t),
231+
Digest: tt.digest,
232+
}, conf.Path); err != nil {
233+
t.Fatalf("putMetadata() err = %v", err)
234+
}
235+
}
236+
237+
got, err := vuls2.HasNewerRemote(context.Background(), conf)
238+
if (err != nil) != tt.wantErr {
239+
t.Fatalf("hasNewerRemote() error = %v, wantErr %v", err, tt.wantErr)
240+
}
241+
if got != tt.want {
242+
t.Errorf("hasNewerRemote() = %v, want %v", got, tt.want)
243+
}
244+
})
245+
}
246+
}

detector/vuls2/export_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
package vuls2
22

3+
import "github.com/MaineK00n/vuls2/pkg/db/session"
4+
5+
// Open exposes Session.open so tests can force the lazy open and inspect what
6+
// it produced.
7+
func (s *Session) Open() (*session.Session, error) { return s.open() }
8+
9+
// SkipsUpdate reports whether this Session is barred from downloading a db, so
10+
// a test can assert that the request path cannot fetch.
11+
func (s *Session) SkipsUpdate() bool { return s.vuls2Conf.SkipUpdate }
12+
313
var (
414
ShouldDownload = shouldDownload
15+
HasNewerRemote = hasNewerRemote
516

617
PreConvertPkgs = preConvertPkgs
718
PreConvertCPEs = preConvertCPEs

0 commit comments

Comments
 (0)