Skip to content

Commit cfdfa18

Browse files
cre8ivejpcre8ivejp
committed
fix: shutdown order
Signed-off-by: Alessandro Yuichi Okimoto <yuichijpn@gmail.com>
1 parent b66f813 commit cfdfa18

File tree

10 files changed

+147
-172
lines changed

10 files changed

+147
-172
lines changed

‎manifests/bucketeer/charts/api/templates/deployment.yaml‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -181,25 +181,31 @@ spec:
181181
- "/bin/sh"
182182
- "-c"
183183
- |
184-
# Fail Envoy health check immediately
184+
# Step 1: Fail Envoy health check so K8s removes pod from endpoints
185185
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/healthcheck/fail
186186
187-
# Wait for API to signal ready for shutdown (max 22s)
188-
# This is coordinated with the app's 20s shutdown timeout.
189-
# Envoy must wait LONGER than the app timeout to ensure it doesn't
190-
# start draining while the app is still processing requests.
191-
timeout=22
192-
while [ $timeout -gt 0 ]; do
193-
if wget -q -O- --no-check-certificate https://localhost:9090/internal/shutdown-ready 2>/dev/null | grep -q "ready"; then
194-
echo "API ready for shutdown, draining connections..."
187+
# Step 2: Stop accepting new inbound connections at Envoy
188+
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/drain_listeners?inboundonly
189+
190+
# Step 3: Wait for all active connections to drain (max 25s)
191+
# Uses Istio pattern: dynamically checks all connections excluding Envoy and TIME-WAIT
192+
elapsed=0
193+
max_wait=25
194+
while [ $elapsed -lt $max_wait ]; do
195+
# Count active connections excluding Envoy process and TIME-WAIT states
196+
active_conns=$(ss -Htlp state all | grep -vE '(envoy|TIME-WAIT)' | wc -l | xargs)
197+
if [ "$active_conns" -eq 0 ]; then
198+
echo "All connections drained after ${elapsed}s"
195199
break
196200
fi
201+
echo "Waiting for $active_conns connections to drain..."
197202
sleep 1
198-
timeout=$((timeout-1))
203+
elapsed=$((elapsed + 1))
199204
done
200205
201-
# Additional drain time for remaining connections
202-
sleep 3
206+
if [ $elapsed -ge $max_wait ]; then
207+
echo "Timeout reached, forcing shutdown with $active_conns remaining connections"
208+
fi
203209
command: ["envoy"]
204210
args:
205211
- "-c"

‎manifests/bucketeer/charts/api/templates/envoy-configmap.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ data:
201201
- name: envoy.filters.http.cors
202202
typed_config:
203203
"@type": type.googleapis.com/envoy.extensions.filters.http.cors.v3.Cors
204+
# DEPRECATED: grpc-web filter for legacy Node.js SDK
205+
# TODO: Remove once Node.js SDK migrates to gRPC-Gateway (REST) or pure gRPC
204206
- name: envoy.filters.http.grpc_web
205207
typed_config:
206208
"@type": type.googleapis.com/envoy.extensions.filters.http.grpc_web.v3.GrpcWeb

‎manifests/bucketeer/charts/batch/templates/deployment.yaml‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -217,25 +217,31 @@ spec:
217217
- "/bin/sh"
218218
- "-c"
219219
- |
220-
# Fail Envoy health check immediately
220+
# Step 1: Fail Envoy health check so K8s removes pod from endpoints
221221
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/healthcheck/fail
222222
223-
# Wait for Batch service to signal ready for shutdown (max 22s)
224-
# This is coordinated with the app's 20s shutdown timeout.
225-
# Envoy must wait LONGER than the app timeout to ensure it doesn't
226-
# start draining while the app is still processing requests.
227-
timeout=22
228-
while [ $timeout -gt 0 ]; do
229-
if wget -q -O- --no-check-certificate https://localhost:9090/internal/shutdown-ready 2>/dev/null | grep -q "ready"; then
230-
echo "Batch service ready for shutdown, draining connections..."
223+
# Step 2: Stop accepting new inbound connections at Envoy
224+
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/drain_listeners?inboundonly
225+
226+
# Step 3: Wait for all active connections to drain (max 25s)
227+
# Uses Istio pattern: dynamically checks all connections excluding Envoy and TIME-WAIT
228+
elapsed=0
229+
max_wait=25
230+
while [ $elapsed -lt $max_wait ]; do
231+
# Count active connections excluding Envoy process and TIME-WAIT states
232+
active_conns=$(ss -Htlp state all | grep -vE '(envoy|TIME-WAIT)' | wc -l | xargs)
233+
if [ "$active_conns" -eq 0 ]; then
234+
echo "All connections drained after ${elapsed}s"
231235
break
232236
fi
237+
echo "Waiting for $active_conns connections to drain..."
233238
sleep 1
234-
timeout=$((timeout-1))
239+
elapsed=$((elapsed + 1))
235240
done
236241
237-
# Additional drain time for remaining connections
238-
sleep 3
242+
if [ $elapsed -ge $max_wait ]; then
243+
echo "Timeout reached, forcing shutdown with $active_conns remaining connections"
244+
fi
239245
command: ["envoy"]
240246
args:
241247
- "-c"

‎manifests/bucketeer/charts/subscriber/templates/deployment.yaml‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -197,27 +197,31 @@ spec:
197197
- "/bin/sh"
198198
- "-c"
199199
- |
200-
# Fail Envoy health check immediately
200+
# Step 1: Fail Envoy health check so K8s removes pod from endpoints
201201
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/healthcheck/fail
202202
203-
# For subscriber service, give time for PubSub message processing to complete (max 22s)
204-
# This is coordinated with the app's 20s shutdown timeout.
205-
# Envoy must wait LONGER than the app timeout to ensure it doesn't exit
206-
# while the subscriber is still processing messages.
207-
# Note: Subscriber uses process detection instead of /internal/shutdown-ready endpoint
208-
timeout=22
209-
while [ $timeout -gt 0 ]; do
210-
# Check if subscriber main process is still running (processing messages)
211-
if ! pgrep -f "subscriber" > /dev/null; then
212-
echo "Subscriber process completed, ready for shutdown..."
203+
# Step 2: Stop accepting new inbound connections at Envoy
204+
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/drain_listeners?inboundonly
205+
206+
# Step 3: Wait for all active connections to drain (max 25s)
207+
# Uses Istio pattern: dynamically checks all connections excluding Envoy and TIME-WAIT
208+
elapsed=0
209+
max_wait=25
210+
while [ $elapsed -lt $max_wait ]; do
211+
# Count active connections excluding Envoy process and TIME-WAIT states
212+
active_conns=$(ss -Htlp state all | grep -vE '(envoy|TIME-WAIT)' | wc -l | xargs)
213+
if [ "$active_conns" -eq 0 ]; then
214+
echo "All connections drained after ${elapsed}s"
213215
break
214216
fi
217+
echo "Waiting for $active_conns connections to drain..."
215218
sleep 1
216-
timeout=$((timeout-1))
219+
elapsed=$((elapsed + 1))
217220
done
218221
219-
# Additional drain time for remaining connections
220-
sleep 3
222+
if [ $elapsed -ge $max_wait ]; then
223+
echo "Timeout reached, forcing shutdown with $active_conns remaining connections"
224+
fi
221225
command: ["envoy"]
222226
args:
223227
- "-c"

‎manifests/bucketeer/charts/web/templates/deployment.yaml‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -267,25 +267,31 @@ spec:
267267
- "/bin/sh"
268268
- "-c"
269269
- |
270-
# Fail Envoy health check immediately
270+
# Step 1: Fail Envoy health check so K8s removes pod from endpoints
271271
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/healthcheck/fail
272272
273-
# Wait for Web services to signal ready for shutdown (max 22s)
274-
# This is coordinated with the app's 20s shutdown timeout.
275-
# Envoy must wait LONGER than the app timeout to ensure it doesn't
276-
# start draining while the app is still processing requests.
277-
timeout=22
278-
while [ $timeout -gt 0 ]; do
279-
if wget -q -O- --no-check-certificate https://localhost:9090/internal/shutdown-ready 2>/dev/null | grep -q "ready"; then
280-
echo "Web services ready for shutdown, draining connections..."
273+
# Step 2: Stop accepting new inbound connections at Envoy
274+
wget -O- --post-data='{}' http://localhost:$ENVOY_ADMIN_PORT/drain_listeners?inboundonly
275+
276+
# Step 3: Wait for all active connections to drain (max 25s)
277+
# Uses Istio pattern: dynamically checks all connections excluding Envoy and TIME-WAIT
278+
elapsed=0
279+
max_wait=25
280+
while [ $elapsed -lt $max_wait ]; do
281+
# Count active connections excluding Envoy process and TIME-WAIT states
282+
active_conns=$(ss -Htlp state all | grep -vE '(envoy|TIME-WAIT)' | wc -l | xargs)
283+
if [ "$active_conns" -eq 0 ]; then
284+
echo "All connections drained after ${elapsed}s"
281285
break
282286
fi
287+
echo "Waiting for $active_conns connections to drain..."
283288
sleep 1
284-
timeout=$((timeout-1))
289+
elapsed=$((elapsed + 1))
285290
done
286291
287-
# Additional drain time for remaining connections
288-
sleep 3
292+
if [ $elapsed -ge $max_wait ]; then
293+
echo "Timeout reached, forcing shutdown with $active_conns remaining connections"
294+
fi
289295
command: ["envoy"]
290296
args:
291297
- "-c"

‎manifests/bucketeer/charts/web/templates/envoy-configmap.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,8 @@ data:
779779
- name: envoy.filters.http.cors
780780
typed_config:
781781
"@type": type.googleapis.com/envoy.extensions.filters.http.cors.v3.Cors
782+
# DEPRECATED: grpc-web filter for legacy Node.js SDK
783+
# TODO: Remove once Node.js SDK migrates to gRPC-Gateway (REST) or pure gRPC
782784
- name: envoy.filters.http.grpc_web
783785
typed_config:
784786
"@type": type.googleapis.com/envoy.extensions.filters.http.grpc_web.v3.GrpcWeb

‎pkg/api/cmd/server.go‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -569,8 +569,12 @@ func (s *server) Run(ctx context.Context, metrics metrics.Metrics, logger *zap.L
569569

570570
// Graceful shutdown sequence optimized for GCP Spot VM constraints (30s termination window):
571571
// 1. Stop health checks immediately to fail Kubernetes readiness probe ASAP
572-
// 2. Gracefully drain all servers in parallel (allows in-flight requests to complete)
573-
// 3. Close clients
572+
// 2. Gracefully drain REST/HTTP servers first (apiGateway + httpServer)
573+
// 3. Then stop gRPC server (after REST traffic completes)
574+
// 4. Close clients
575+
//
576+
// Shutdown order is critical because apiGateway forwards requests to server (port 9090).
577+
// If server stops while apiGateway is processing, those requests fail.
574578
//
575579
// This coordinates with Envoy's preStop hook which waits for /internal/shutdown-ready
576580
// to return 200 (set by rpc.Server after graceful shutdown completes).
@@ -581,16 +585,10 @@ func (s *server) Run(ctx context.Context, metrics metrics.Metrics, logger *zap.L
581585
healthChecker.Stop()
582586
restHealthChecker.Stop()
583587

584-
// Step 2: Gracefully stop all servers in parallel
585-
// Each server will reject new requests and wait for existing requests to complete.
588+
// Step 2: Gracefully stop REST/HTTP servers (these call the gRPC server internally)
589+
// We run these in parallel since they don't depend on each other
586590
var wg sync.WaitGroup
587591

588-
wg.Add(1)
589-
go func() {
590-
defer wg.Done()
591-
server.Stop(serverShutDownTimeout)
592-
}()
593-
594592
wg.Add(1)
595593
go func() {
596594
defer wg.Done()
@@ -603,9 +601,12 @@ func (s *server) Run(ctx context.Context, metrics metrics.Metrics, logger *zap.L
603601
httpServer.Stop(serverShutDownTimeout)
604602
}()
605603

606-
// Wait for all servers to complete shutdown
604+
// Wait for REST/HTTP traffic to drain
607605
wg.Wait()
608606

607+
// Step 3: Stop gRPC server (only pure gRPC connections remain)
608+
server.Stop(serverShutDownTimeout)
609+
609610
// Step 3: Close clients
610611
// These are fast cleanup operations that can run asynchronously.
611612
go goalPublisher.Stop()

‎pkg/batch/cmd/server/server.go‎

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"fmt"
2020
"os"
2121
"strings"
22-
"sync"
2322
"time"
2423

2524
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
@@ -621,8 +620,12 @@ func (s *server) Run(ctx context.Context, metrics metrics.Metrics, logger *zap.L
621620

622621
// Graceful shutdown sequence optimized for GCP Spot VM constraints (30s termination window):
623622
// 1. Stop health check immediately to fail Kubernetes readiness probe ASAP
624-
// 2. Gracefully drain all servers in parallel (allows in-flight requests to complete)
625-
// 3. Close database/cache/pubsub clients
623+
// 2. Gracefully drain REST gateway first (batchGateway)
624+
// 3. Then stop gRPC server (after REST traffic completes)
625+
// 4. Close database/cache/pubsub clients
626+
//
627+
// Shutdown order is critical because batchGateway forwards requests to server (port 9000).
628+
// If server stops while batchGateway is processing, those requests fail.
626629
//
627630
// This coordinates with Envoy's preStop hook which waits for /internal/shutdown-ready
628631
// to return 200 (set by rpc.Server after graceful shutdown completes).
@@ -632,24 +635,11 @@ func (s *server) Run(ctx context.Context, metrics metrics.Metrics, logger *zap.L
632635
// preventing new traffic from being routed to this pod.
633636
healthChecker.Stop()
634637

635-
// Step 2: Gracefully stop all servers in parallel
636-
// Each server will reject new requests and wait for existing requests to complete.
637-
var wg sync.WaitGroup
638-
639-
wg.Add(1)
640-
go func() {
641-
defer wg.Done()
642-
server.Stop(serverShutDownTimeout)
643-
}()
644-
645-
wg.Add(1)
646-
go func() {
647-
defer wg.Done()
648-
batchGateway.Stop(serverShutDownTimeout)
649-
}()
638+
// Step 2: Gracefully stop REST gateway (calls the gRPC server internally)
639+
batchGateway.Stop(serverShutDownTimeout)
650640

651-
// Wait for all servers to complete shutdown
652-
wg.Wait()
641+
// Step 3: Stop gRPC server (only pure gRPC connections remain)
642+
server.Stop(serverShutDownTimeout)
653643

654644
// Step 3: Close clients
655645
// These are fast cleanup operations that can run asynchronously.

0 commit comments

Comments
 (0)