Skip to content

Commit

Permalink
honor offline caching in /v1/status API (#412)
Browse files Browse the repository at this point in the history
Now, the `/v1/status` API will not try fetch the backend
keystore status if:
 - offline caching is enabled.
 - the keystore is considered offline.

The idea here is that if the cache considered the backend
keystore as offline then it is unlikely that a `Status`
suceeds. Instead, we can directly return unreachable.

Once, the keystore becomes available again, the cache will
detect it eventually (on its next health check cycle).

Among other things, this improves pod availability when
`/v1/status` is used as (part of) readiness probes. (i.e. MinIO)
Such probes are now less likely to time out since `/v1/status`
returns immediately.
However, if the backend keystore becomes unavailable and a
`/v1/status` request arrives BEFORE the cache detects that
the keystore is offline then these requests may time out.
Hence, it is recommended to use readiness probes with a retry
threshold > 1.

Signed-off-by: Andreas Auernhammer <[email protected]>
  • Loading branch information
aead authored Oct 31, 2023
1 parent ce11734 commit 3ee893d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
1 change: 0 additions & 1 deletion internal/api/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ type StatusResponse struct {
StackAlloc uint64 `json:"mem_stack_used"`

KeyStoreLatency int64 `json:"keystore_latency,omitempty"` // In microseconds
KeyStoreUnavailable bool `json:"keystore_unavailable,omitempty"`
KeyStoreUnreachable bool `json:"keystore_unreachable,omitempty"`
}

Expand Down
7 changes: 7 additions & 0 deletions keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/minio/kes-go"
"github.com/minio/kes/internal/cache"
"github.com/minio/kes/internal/key"
"github.com/minio/kes/kv"
)

// A KeyStore stores key-value pairs. It provides durable storage for a
Expand Down Expand Up @@ -238,7 +239,13 @@ type cacheEntry struct {
}

// Status returns the current state of the underlying KeyStore.
//
// It immediately returns an error if the backend keystore is not
// reachable and offline caching is enabled.
func (c *keyCache) Status(ctx context.Context) (KeyStoreState, error) {
if c.offline.Load() {
return KeyStoreState{}, &kv.Unreachable{Err: errors.New("keystore is offline")}
}
return c.store.Status(ctx)
}

Expand Down
25 changes: 12 additions & 13 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,27 +477,27 @@ func (s *Server) ready(resp *api.Response, req *api.Request) {
}

func (s *Server) status(resp *api.Response, req *api.Request) {
info, err := sys.ReadBinaryInfo()
if err != nil {
s.state.Load().Log.ErrorContext(req.Context(), err.Error(), "req", req)
resp.Fail(http.StatusInternalServerError, "failed to read server version")
return
}

var (
unavailable, unreachable bool
latency time.Duration
latency time.Duration
unreachable = true
)
state, err := s.state.Load().Keys.Status(req.Context())
if err != nil {
unavailable = true
_, unreachable = kv.IsUnreachable(err)
} else {
if err == nil {
unreachable = false
latency = state.Latency.Round(time.Millisecond)

if latency == 0 { // Make sure we actually send a latency even if the key store respond time is < 1ms.
latency = 1 * time.Millisecond
}
}

info, err := sys.ReadBinaryInfo()
if err != nil {
s.state.Load().Log.ErrorContext(req.Context(), err.Error(), "req", req)
resp.Fail(http.StatusInternalServerError, "failed to read server version")
return
}
var memStats runtime.MemStats
runtime.ReadMemStats(&memStats)

Expand All @@ -513,7 +513,6 @@ func (s *Server) status(resp *api.Response, req *api.Request) {
StackAlloc: memStats.StackSys,

KeyStoreLatency: latency.Milliseconds(),
KeyStoreUnavailable: unavailable,
KeyStoreUnreachable: unreachable,
})
}
Expand Down

0 comments on commit 3ee893d

Please sign in to comment.