Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Server shutdown grace period #390

Merged
5 commits merged into from Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/open_saves.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions api/open_saves_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion configs/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ open_saves_cloud: "gcp"
open_saves_bucket: ""
open_saves_project: ""
log_level: "info"
shutdown_grace_period: "5s"
cache_default_ttl: "5m"

redis_address: "localhost:6379"
Expand All @@ -13,4 +14,4 @@ redis_max_retries: 3
redis_min_retry_backoff: "8ms"
redis_max_retry_backoff: "512ms"

blob_max_inline_size: 65536
blob_max_inline_size: 65536
14 changes: 10 additions & 4 deletions internal/app/server/open_saves_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,11 @@ import (
)

const (
testProject = "triton-for-games-dev"
testBucket = "gs://triton-integration"
testPort = "8000"
testBufferSize = 1024 * 1024
testProject = "triton-for-games-dev"
testBucket = "gs://triton-integration"
testPort = "8000"
testBufferSize = 1024 * 1024
testShutdownGracePeriod = "1s"
// The threshold of comparing times.
// Since the server will actually access the backend datastore,
// we need enough time to prevent flaky tests.
Expand All @@ -75,6 +76,7 @@ func getServiceConfig() (*config.ServiceConfig, error) {
viper.Set(config.OpenSavesBucket, testBucket)
viper.Set(config.OpenSavesProject, testProject)
viper.Set(config.OpenSavesPort, testPort)
viper.Set(config.ShutdownGracePeriod, testShutdownGracePeriod)
sc, err := config.Load(configPath)
serviceConfig = sc
return sc, err
Expand Down Expand Up @@ -124,6 +126,8 @@ func TestOpenSaves_HealthCheck(t *testing.T) {
if want := healthgrpc.HealthCheckResponse_NOT_SERVING; got.Status != want {
t.Errorf("hc.Check got: %v, want: %v", got.Status, want)
}
// Sleep long enough to free up the connection
time.Sleep(serviceConfig.ShutdownGracePeriod)
}

func TestOpenSaves_RunServer(t *testing.T) {
Expand All @@ -148,6 +152,8 @@ func TestOpenSaves_RunServer(t *testing.T) {
t.Errorf("got err calling server.Run: %v", err)
}
})
// Sleep long enough to free up the connection
time.Sleep(serviceConfig.ShutdownGracePeriod)
}

func getOpenSavesServer(ctx context.Context, t *testing.T, cloud string) (*openSavesServer, *bufconn.Listener) {
Expand Down
8 changes: 7 additions & 1 deletion internal/app/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"os"
"os/signal"
"syscall"
"time"

"github.com/googleforgames/open-saves/internal/pkg/config"

Expand Down Expand Up @@ -65,8 +66,13 @@ func Run(ctx context.Context, network string, cfg *config.ServiceConfig) error {
case <-ctx.Done():
log.Infoln("context cancelled")
}
log.Infoln("stopping open saves server gracefully")
log.Infoln("set health check to not serving")
healthcheck.SetServingStatus(serviceName, healthgrpc.HealthCheckResponse_NOT_SERVING)

log.Infoln("starting server shutdown grace period")
time.Sleep(cfg.ShutdownGracePeriod)

log.Infoln("stopping open saves server gracefully")
s.GracefulStop()
log.Infoln("stopped open saves server gracefully")
}()
Expand Down
9 changes: 5 additions & 4 deletions internal/pkg/config/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,11 @@ func Load(path string) (*ServiceConfig, error) {
}

serverConfig := ServerConfig{
Address: fmt.Sprintf(":%d", viper.GetUint(OpenSavesPort)),
Cloud: viper.GetString(OpenSavesCloud),
Bucket: viper.GetString(OpenSavesBucket),
Project: viper.GetString(OpenSavesProject),
Address: fmt.Sprintf(":%d", viper.GetUint(OpenSavesPort)),
Cloud: viper.GetString(OpenSavesCloud),
Bucket: viper.GetString(OpenSavesBucket),
Project: viper.GetString(OpenSavesProject),
ShutdownGracePeriod: viper.GetDuration(ShutdownGracePeriod),
}

// Cloud Run environment populates the PORT env var, so check for it here.
Expand Down
20 changes: 11 additions & 9 deletions internal/pkg/config/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ package config
import "time"

const (
OpenSavesPort = "open_saves_port"
OpenSavesCloud = "open_saves_cloud"
OpenSavesBucket = "open_saves_bucket"
OpenSavesProject = "open_saves_project"
LogLevel = "log_level"
OpenSavesPort = "open_saves_port"
OpenSavesCloud = "open_saves_cloud"
OpenSavesBucket = "open_saves_bucket"
OpenSavesProject = "open_saves_project"
LogLevel = "log_level"
ShutdownGracePeriod = "shutdown_grace_period"

CacheDefaultTTL = "cache_default_ttl"

Expand All @@ -45,10 +46,11 @@ type ServiceConfig struct {

// ServerConfig defines common fields needed to start Open Saves.
type ServerConfig struct {
Address string
Cloud string
Bucket string
Project string
Address string
Cloud string
Bucket string
Project string
ShutdownGracePeriod time.Duration
}

// CacheConfig has configurations for caching control (not Redis specific).
Expand Down