diff --git a/cmd/objgitd/main.go b/cmd/objgitd/main.go index f5f0c9d..c81bd6c 100644 --- a/cmd/objgitd/main.go +++ b/cmd/objgitd/main.go @@ -48,6 +48,9 @@ var ( s3CacheRecursive = flag.String("s3-cache-recursive-prefixes", "refs/", "comma-separated key prefixes served from one recursive subtree scan instead of a listing per folder; empty disables subtree caching") s3CacheMaxSubtree = flag.Int("s3-cache-max-subtree-keys", 50000, "abandon a recursive subtree scan past this many keys and fall back to per-folder listing") + + packCacheBytes = flag.Int64("pack-cache-bytes", 2<<30, "local disk budget for cached pack files (.pack/.idx/.rev), downloaded once and served from a temp file so a clone doesn't re-fetch pack objects per access; 0 disables the cache") + packCacheDir = flag.String("pack-cache-dir", "", "parent directory for the pack-file cache; empty uses the OS temp dir") ) func main() { @@ -77,11 +80,15 @@ func main() { // Route s3fs S3 round-trips into Prometheus before any filesystem use. s3fs.SetMetricsObserver(metrics.ObserveS3) - client, err := storage.New(ctx) + rawClient, err := storage.New(ctx) if err != nil { slog.Error("can't create Tigris storage client", "err", err) os.Exit(1) } + // Harden the client's HTTP path so stale keep-alive connections to Tigris + // fail fast and retry on a fresh connection instead of hanging the request + // forever (see internal/s3fs/resilient.go). + client := s3fs.Harden(rawClient) var cache *s3fs.ListingCache var fsOpts []s3fs.Option @@ -109,6 +116,16 @@ func main() { }) } + if *packCacheBytes != 0 { + packCache, err := s3fs.NewPackCache(*packCacheDir, *packCacheBytes) + if err != nil { + slog.Error("can't create pack cache", "err", err) + os.Exit(1) + } + defer packCache.Cleanup() + fsOpts = append(fsOpts, s3fs.WithPackCache(packCache)) + } + fsys, err := s3fs.NewS3FS(client, *bucket, fsOpts...) if err != nil { slog.Error("can't create s3fs", "bucket", *bucket, "err", err) diff --git a/docs/plans/pack-temp-file-cache.md b/docs/plans/pack-temp-file-cache.md new file mode 100644 index 0000000..f2c9092 --- /dev/null +++ b/docs/plans/pack-temp-file-cache.md @@ -0,0 +1,73 @@ +# Local temp-file pack cache + +## Problem + +Serving a clone runs go-git's `upload-pack`, whose delta-compression phase +(`deltaSelector.ObjectsToPack` → `Encoder.Encode`) re-reads the repository's pack +objects **thousands of times** with random access. Commit `76bb55d` made pack +reads lazy: every object access issues a fresh S3 `GetObject`. Measured on a +318-object repo (200 KB pack), serving one clone made **8,500+ GetObject calls +and climbing** — the clone never finishes within any reasonable timeout. Real +repos (kefka: 19.6 MB pack, far more objects) are effectively unservable. + +The old eager reader buffered the whole pack in RAM once, so the thousands of +re-reads were in-memory and instant — at the cost of holding whole packs in RAM, +which `76bb55d` set out to avoid. + +A secondary failure rode on top: among those thousands of GetObjects, some reused +stale keep-alive connections to Tigris and, with `context.TODO()` (no timeout), +hung forever. That is fixed separately by `internal/s3fs/resilient.go` (hardened +HTTP client). This plan addresses the round-trip explosion. + +## Approach + +Materialise each pack-directory file (`.pack`, `.idx`, `.rev`) to a **local temp +file** on first open, and serve all reads from that local file. This gives +go-git cheap repeated random access (local disk) without buffering whole packs in +RAM. The user explicitly chose this over RAM buffering, accepting the local-disk +dependency the project otherwise avoids. + +Pack-dir files are immutable and content-addressed (`pack-.pack`), so a +downloaded temp file is valid for the file's whole lifetime and safe to cache by +S3 key. + +## Design + +`internal/s3fs/packcache.go`: + +- `PackCache` — keyed by S3 object key. Each entry downloads its object once + (`sync.Once`) into a temp file under a per-process temp dir, then records the + local path + size. A total-bytes LRU budget evicts least-recently-opened + entries (`os.Remove`; open readers keep working — Linux unlinked-while-open). +- `open(ctx, client, bucket, key, name)` returns a `packCachedFile` wrapping an + **independent** `*os.File` (`os.Open` of the cached path, so each reader has its + own seek cursor). `Close` releases the fd but keeps the cached temp file. +- `Cleanup()` removes the temp dir; `main` defers it. +- Download streams `GetObject` (full object) → temp file via `io.Copy`. NoSuchKey + maps to `fs.ErrNotExist` (matches `newS3ReadFile`). + +`packCachedFile` embeds `*os.File` (Read/ReadAt/Seek/Close/Stat) and adds billy's +`Lock`/`Unlock` (no-ops); `Write`/`WriteAt`/`Truncate` return read-only errors; +`Name` returns the logical path. + +Wiring: + +- `S3FS.packCache *PackCache`, propagated in `Chroot` (shared by pointer like + `cache`). `WithPackCache(*PackCache) Option`. +- `OpenFile` `O_RDONLY`: after the temp/dir short-circuits, if `packCache != nil` + and the key is a pack-dir file, return `packCache.open(...)` instead of + `newS3ReadFile`. +- `main.go`: `-pack-cache-bytes` (default 2 GiB; 0 disables) and + `-pack-cache-dir` (default `os.TempDir()`); construct `PackCache`, wire via + `WithPackCache`, `defer Cleanup()`. + +## Tests + +- `packcache_test.go` (table-driven, stub `s3Client` serving in-memory bytes): + download-once (N opens → 1 GetObject), independent seek cursors across + concurrent readers, correct bytes via Read and ReadAt, ErrClosed after Close, + NoSuchKey → ErrNotExist, LRU eviction frees disk while keeping an already-open + reader valid. +- End-to-end: clone the 318-object repo and kefka through the server; assert the + GetObject count is small (≈ pack-dir file count, not thousands) and `git fsck` + passes. diff --git a/internal/s3fs/basic.go b/internal/s3fs/basic.go index 564ad0f..0ae5356 100644 --- a/internal/s3fs/basic.go +++ b/internal/s3fs/basic.go @@ -74,6 +74,15 @@ func (fs3 *S3FS) OpenFile(filename string, flag int, perm os.FileMode) (billy.Fi return &tempReadFile{buf: buf, name: filename}, nil } + // Immutable pack-directory files are re-read with random access + // thousands of times while serving a clone. Download each once to a + // local temp file and serve from disk, instead of one S3 round-trip per + // access (which makes clones of real repos never finish). See + // docs/plans/pack-temp-file-cache.md. + if fs3.packCache != nil && isPackCacheable(key) { + return fs3.packCache.open(context.TODO(), fs3.client, fs3.bucket, key, filename) + } + // If the parent folder's listing is cached, resolve the open without a // negotiation round-trip: absent → not-exist, a sub-prefix → directory. // For a present file the head cache (seeded from the listing) lets diff --git a/internal/s3fs/chroot.go b/internal/s3fs/chroot.go index cac6eeb..34080a6 100644 --- a/internal/s3fs/chroot.go +++ b/internal/s3fs/chroot.go @@ -25,6 +25,7 @@ func (fs3 *S3FS) Chroot(path string) (billy.Filesystem, error) { separator: fs3.separator, unixMeta: fs3.unixMeta, cache: fs3.cache, + packCache: fs3.packCache, temps: make(map[string]*tempBuffer), } return nfs, nil diff --git a/internal/s3fs/filesystem.go b/internal/s3fs/filesystem.go index 1877a78..aac8bed 100644 --- a/internal/s3fs/filesystem.go +++ b/internal/s3fs/filesystem.go @@ -51,6 +51,11 @@ type S3FS struct { // shared by pointer across this filesystem and all of its Chroot children. cache *ListingCache + // packCache, when non-nil, serves reads of immutable pack-directory files + // (.pack/.idx/.rev) from a local temp file downloaded once, instead of one + // S3 round-trip per object access. Shared by pointer across Chroot children. + packCache *PackCache + // temps holds TempFile-backed buffers keyed by canonical S3 key, so a // subsequent Open of the same path returns a reader over the same bytes // the writer is still appending to. See tempfs.go. @@ -81,6 +86,15 @@ func WithListingCache(c *ListingCache) Option { } } +// WithPackCache attaches a local temp-file cache for immutable pack-directory +// files. The same *PackCache is carried into every Chroot child so the whole +// tree shares it. Construct it with NewPackCache and defer its Cleanup. +func WithPackCache(c *PackCache) Option { + return func(fs3 *S3FS) { + fs3.packCache = c + } +} + // NewS3FS creates a new S3FS Filesystem. client is typically a *storage.Client; // it is accepted as the s3Client interface so tests can substitute a stub. func NewS3FS(client s3Client, bucket string, opts ...Option) (billy.Filesystem, error) { diff --git a/internal/s3fs/packcache.go b/internal/s3fs/packcache.go new file mode 100644 index 0000000..8a55ff9 --- /dev/null +++ b/internal/s3fs/packcache.go @@ -0,0 +1,212 @@ +package s3fs + +import ( + "context" + "fmt" + "io" + "io/fs" + "os" + "strings" + "sync" + "time" + + "github.com/aws/aws-sdk-go-v2/service/s3" +) + +// isPackCacheable reports whether key names an immutable pack-directory file +// that benefits from the local temp-file cache. These files are content- +// addressed (pack-.{pack,idx,rev}) and re-read with random access many +// times while serving a clone, so a single download served from local disk +// replaces thousands of S3 round-trips. See docs/plans/pack-temp-file-cache.md. +func isPackCacheable(key string) bool { + return strings.HasSuffix(key, ".pack") || + strings.HasSuffix(key, ".idx") || + strings.HasSuffix(key, ".rev") +} + +// PackCache materialises immutable pack-directory objects to local temp files +// and serves their reads from disk. go-git's upload-pack re-reads pack objects +// thousands of times during delta compression; without this each access is a +// fresh S3 GetObject and a clone never completes. The cache is shared by pointer +// across an S3FS and all of its Chroot children. +// +// Entries are keyed by S3 object key. Each downloads its object once and is +// reused across opens; a total-bytes budget evicts the least-recently-opened +// entries. Eviction unlinks the temp file, which on Linux leaves already-open +// readers working until they close, so eviction never corrupts an in-flight +// read. +type PackCache struct { + dir string + maxBytes int64 + + mu sync.Mutex + entries map[string]*packEntry + curBytes int64 + seq uint64 // monotonic open counter; entry.used orders the LRU +} + +// packEntry is one cached object. once guards the single download; path/size/err +// are set by it. used is the seq of the most recent open, for LRU ordering. +type packEntry struct { + key string + once sync.Once + path string + size int64 + err error + used uint64 +} + +// NewPackCache creates a pack cache writing temp files under a fresh directory +// inside parent (os.TempDir() when empty). maxBytes bounds the total size of +// cached files; opens past the budget evict the least-recently-opened entries. +// A maxBytes <= 0 disables the budget (no eviction). Call Cleanup to remove the +// temp directory. +func NewPackCache(parent string, maxBytes int64) (*PackCache, error) { + if parent == "" { + parent = os.TempDir() + } + dir, err := os.MkdirTemp(parent, "objgit-packs-") + if err != nil { + return nil, fmt.Errorf("s3fs: create pack cache dir: %w", err) + } + return &PackCache{ + dir: dir, + maxBytes: maxBytes, + entries: make(map[string]*packEntry), + }, nil +} + +// Cleanup removes the cache's temp directory and all files in it. Already-open +// readers keep working (unlinked-while-open); new opens after Cleanup fail. +func (c *PackCache) Cleanup() error { + c.mu.Lock() + dir := c.dir + c.entries = map[string]*packEntry{} + c.curBytes = 0 + c.mu.Unlock() + return os.RemoveAll(dir) +} + +// open returns a billy.File for key, downloading the object to a temp file on +// first use and serving from that file thereafter. Each call returns an +// independent *os.File handle so concurrent readers have their own seek cursor. +func (c *PackCache) open(ctx context.Context, client s3Client, bucket, key, name string) (*packCachedFile, error) { + c.mu.Lock() + e := c.entries[key] + if e == nil { + e = &packEntry{key: key} + c.entries[key] = e + } + c.mu.Unlock() + + e.once.Do(func() { + path, size, err := c.download(ctx, client, bucket, key) + if err != nil { + e.err = err + // Drop the failed entry so a later open retries the download. + c.mu.Lock() + if c.entries[key] == e { + delete(c.entries, key) + } + c.mu.Unlock() + return + } + e.path, e.size = path, size + c.mu.Lock() + c.curBytes += size + c.evictLocked(key) + c.mu.Unlock() + }) + if e.err != nil { + return nil, e.err + } + + f, err := os.Open(e.path) + if err != nil { + // The cached file was evicted/cleaned between download and open; retry + // through a fresh entry so the object is fetched again. + c.mu.Lock() + if c.entries[key] == e { + delete(c.entries, key) + } + c.mu.Unlock() + return nil, err + } + + c.mu.Lock() + c.seq++ + e.used = c.seq + c.mu.Unlock() + + return &packCachedFile{File: f, name: name}, nil +} + +// download streams the full object to a temp file and returns its path and size. +func (c *PackCache) download(ctx context.Context, client s3Client, bucket, key string) (string, int64, error) { + start := time.Now() + out, err := client.GetObject(ctx, &s3.GetObjectInput{Bucket: &bucket, Key: &key}) + observeS3("GetObject", start, err) + if err != nil { + if isNotFound(err) { + return "", 0, &os.PathError{Op: "open", Path: key, Err: fs.ErrNotExist} + } + return "", 0, fmt.Errorf("pack cache GetObject %q: %w", key, err) + } + defer out.Body.Close() + + tmp, err := os.CreateTemp(c.dir, "obj-") + if err != nil { + return "", 0, fmt.Errorf("pack cache temp file: %w", err) + } + n, err := io.Copy(tmp, out.Body) + if cerr := tmp.Close(); err == nil { + err = cerr + } + if err != nil { + os.Remove(tmp.Name()) + return "", 0, fmt.Errorf("pack cache download %q: %w", key, err) + } + return tmp.Name(), n, nil +} + +// evictLocked removes least-recently-opened entries until the cache is within +// budget. keep is never evicted (it is the entry the caller just populated). +// Callers hold c.mu. A non-positive maxBytes disables eviction. +func (c *PackCache) evictLocked(keep string) { + if c.maxBytes <= 0 { + return + } + for c.curBytes > c.maxBytes { + var victim *packEntry + for _, e := range c.entries { + if e.key == keep || e.path == "" { + continue + } + if victim == nil || e.used < victim.used { + victim = e + } + } + if victim == nil { + return // nothing evictable + } + os.Remove(victim.path) // open readers survive on Linux (unlinked fd) + c.curBytes -= victim.size + delete(c.entries, victim.key) + } +} + +// packCachedFile is a read-only billy.File backed by a local temp file. It +// embeds *os.File for Read/ReadAt/Seek/Close/Stat and supplies the billy-only +// Lock/Unlock; writes are rejected. +type packCachedFile struct { + *os.File + name string +} + +func (f *packCachedFile) Name() string { return f.name } + +func (f *packCachedFile) Write(p []byte) (int, error) { return 0, ErrCantWriteToReadOnly } +func (f *packCachedFile) WriteAt(p []byte, off int64) (int, error) { return 0, ErrCantWriteToReadOnly } +func (f *packCachedFile) Truncate(size int64) error { return ErrTruncateNotSupported } +func (f *packCachedFile) Lock() error { return ErrLockNotSupported } +func (f *packCachedFile) Unlock() error { return ErrLockNotSupported } diff --git a/internal/s3fs/packcache_test.go b/internal/s3fs/packcache_test.go new file mode 100644 index 0000000..1f1c966 --- /dev/null +++ b/internal/s3fs/packcache_test.go @@ -0,0 +1,278 @@ +package s3fs + +import ( + "bytes" + "context" + "errors" + "io" + "io/fs" + "os" + "sync" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" +) + +// packStub serves fixed object bytes and counts GetObject calls per key, so a +// test can assert the cache downloads each object exactly once. The embedded nil +// s3Client is never used: PackCache only calls GetObject. +type packStub struct { + s3Client + mu sync.Mutex + objs map[string][]byte + gets map[string]int +} + +func newPackStub(objs map[string][]byte) *packStub { + return &packStub{objs: objs, gets: map[string]int{}} +} + +func (s *packStub) getCount(key string) int { + s.mu.Lock() + defer s.mu.Unlock() + return s.gets[key] +} + +func (s *packStub) GetObject(_ context.Context, in *s3.GetObjectInput, _ ...func(*s3.Options)) (*s3.GetObjectOutput, error) { + s.mu.Lock() + defer s.mu.Unlock() + key := aws.ToString(in.Key) + b, ok := s.objs[key] + if !ok { + return nil, notFound() + } + s.gets[key]++ + return &s3.GetObjectOutput{ + Body: io.NopCloser(bytes.NewReader(b)), + ContentLength: aws.Int64(int64(len(b))), + }, nil +} + +// newTestPackCache builds a PackCache under t.TempDir with the given budget and +// registers Cleanup. +func newTestPackCache(t *testing.T, maxBytes int64) *PackCache { + t.Helper() + pc, err := NewPackCache(t.TempDir(), maxBytes) + if err != nil { + t.Fatalf("NewPackCache: %v", err) + } + t.Cleanup(func() { pc.Cleanup() }) + return pc +} + +func mustReadAll(t *testing.T, f *packCachedFile) []byte { + t.Helper() + if _, err := f.Seek(0, io.SeekStart); err != nil { + t.Fatalf("seek: %v", err) + } + b, err := io.ReadAll(f) + if err != nil { + t.Fatalf("read: %v", err) + } + return b +} + +func TestPackCacheReads(t *testing.T) { + ctx := context.Background() + const key = "repo.git/objects/pack/pack-abc.pack" + const name = "objects/pack/pack-abc.pack" + want := bytes.Repeat([]byte("PACKDATA"), 4096) // 32 KiB + + tests := []struct { + name string + // check exercises one read path and returns (got, wantSlice) to compare. + check func(t *testing.T, f *packCachedFile) (got, exp []byte) + }{ + { + name: "sequential Read", + check: func(t *testing.T, f *packCachedFile) ([]byte, []byte) { + return mustReadAll(t, f), want + }, + }, + { + name: "ReadAt mid-object", + check: func(t *testing.T, f *packCachedFile) ([]byte, []byte) { + p := make([]byte, 100) + if _, err := f.ReadAt(p, 8000); err != nil && err != io.EOF { + t.Fatalf("ReadAt: %v", err) + } + return p, want[8000:8100] + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stub := newPackStub(map[string][]byte{key: want}) + pc := newTestPackCache(t, 0) + + f, err := pc.open(ctx, stub, "bucket", key, name) + if err != nil { + t.Fatalf("open: %v", err) + } + defer f.Close() + + got, exp := tt.check(t, f) + if !bytes.Equal(got, exp) { + t.Errorf("read bytes mismatch: got %d bytes, want %d", len(got), len(exp)) + } + if f.Name() != name { + t.Errorf("Name = %q, want %q", f.Name(), name) + } + }) + } +} + +// TestPackCacheDownloadsOnce confirms repeated opens of the same key hit the +// cache: exactly one GetObject regardless of how many readers open it. This is +// the whole point — replacing thousands of S3 round-trips with one. +func TestPackCacheDownloadsOnce(t *testing.T) { + ctx := context.Background() + const key = "r.git/objects/pack/pack-x.pack" + data := bytes.Repeat([]byte{0xAB}, 1024) + stub := newPackStub(map[string][]byte{key: data}) + pc := newTestPackCache(t, 0) + + const opens = 25 + for i := range opens { + f, err := pc.open(ctx, stub, "bucket", key, "name") + if err != nil { + t.Fatalf("open %d: %v", i, err) + } + if got := mustReadAll(t, f); !bytes.Equal(got, data) { + t.Fatalf("open %d: bytes mismatch", i) + } + f.Close() + } + if n := stub.getCount(key); n != 1 { + t.Errorf("GetObject called %d times across %d opens, want 1", n, opens) + } +} + +// TestPackCacheIndependentCursors verifies two open handles over the same cached +// file have independent seek positions (each gets its own *os.File). +func TestPackCacheIndependentCursors(t *testing.T) { + ctx := context.Background() + const key = "r.git/objects/pack/pack-y.idx" + data := []byte("0123456789abcdef") + stub := newPackStub(map[string][]byte{key: data}) + pc := newTestPackCache(t, 0) + + a, err := pc.open(ctx, stub, "bucket", key, "a") + if err != nil { + t.Fatal(err) + } + defer a.Close() + b, err := pc.open(ctx, stub, "bucket", key, "b") + if err != nil { + t.Fatal(err) + } + defer b.Close() + + if _, err := a.Seek(10, io.SeekStart); err != nil { + t.Fatal(err) + } + pa := make([]byte, 3) + if _, err := io.ReadFull(a, pa); err != nil { + t.Fatal(err) + } + if string(pa) != "abc" { + t.Errorf("a read %q, want abc", pa) + } + // b's cursor is untouched, still at 0. + pb := make([]byte, 3) + if _, err := io.ReadFull(b, pb); err != nil { + t.Fatal(err) + } + if string(pb) != "012" { + t.Errorf("b read %q, want 012 (independent cursor)", pb) + } +} + +func TestPackCacheClosedReadFails(t *testing.T) { + ctx := context.Background() + const key = "r.git/objects/pack/pack-z.pack" + stub := newPackStub(map[string][]byte{key: []byte("data")}) + pc := newTestPackCache(t, 0) + + f, err := pc.open(ctx, stub, "bucket", key, "n") + if err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatalf("close: %v", err) + } + if _, err := f.Read(make([]byte, 1)); !errors.Is(err, os.ErrClosed) { + t.Errorf("read after close: err = %v, want os.ErrClosed", err) + } +} + +func TestPackCacheMissingObject(t *testing.T) { + stub := newPackStub(map[string][]byte{}) + pc := newTestPackCache(t, 0) + + _, err := pc.open(context.Background(), stub, "bucket", "r.git/objects/pack/absent.pack", "n") + if !errors.Is(err, fs.ErrNotExist) { + t.Errorf("open absent: err = %v, want fs.ErrNotExist", err) + } +} + +// TestPackCacheEviction checks that exceeding the byte budget evicts the +// least-recently-opened entry (re-download on next open) while a reader holding +// the evicted file open still reads correct bytes (unlinked-while-open). +func TestPackCacheEviction(t *testing.T) { + ctx := context.Background() + const keyA = "r.git/objects/pack/A.pack" + const keyB = "r.git/objects/pack/B.pack" + dataA := bytes.Repeat([]byte("A"), 1000) + dataB := bytes.Repeat([]byte("B"), 1000) + stub := newPackStub(map[string][]byte{keyA: dataA, keyB: dataB}) + + // Budget holds only one object; opening B evicts A. + pc := newTestPackCache(t, 1500) + + fa, err := pc.open(ctx, stub, "bucket", keyA, "a") + if err != nil { + t.Fatal(err) + } + defer fa.Close() + + // Open B: total (2000) exceeds 1500, so A is evicted from the cache. + fb, err := pc.open(ctx, stub, "bucket", keyB, "b") + if err != nil { + t.Fatal(err) + } + fb.Close() + + // fa was opened before eviction; its fd still reads A's bytes correctly. + if got := mustReadAll(t, fa); !bytes.Equal(got, dataA) { + t.Errorf("evicted-but-open reader: bytes mismatch") + } + + // Re-opening A must re-download (its cache entry was dropped). + fa2, err := pc.open(ctx, stub, "bucket", keyA, "a2") + if err != nil { + t.Fatal(err) + } + fa2.Close() + if n := stub.getCount(keyA); n != 2 { + t.Errorf("A GetObject count = %d, want 2 (re-downloaded after eviction)", n) + } +} + +func TestIsPackCacheable(t *testing.T) { + tests := map[string]bool{ + "r.git/objects/pack/pack-1.pack": true, + "r.git/objects/pack/pack-1.idx": true, + "r.git/objects/pack/pack-1.rev": true, + "r.git/refs/heads/main": false, + "r.git/HEAD": false, + "r.git/config": false, + } + for key, want := range tests { + if got := isPackCacheable(key); got != want { + t.Errorf("isPackCacheable(%q) = %v, want %v", key, got, want) + } + } +}