chore(application-system): Set initialDelay to 20 seconds #16893
Conversation
WalkthroughThe changes in this pull request primarily enhance the configuration of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
norda-gunni
force-pushed
the
as-increase-initial-delay
branch
from
ac37d07 to
4ab931c
Compare
norda-gunni
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
charts/services/application-system-api/values.dev.yaml (1)
145-146: Health check delay increase looks good but consider staggering the probes.The increase to 20 seconds aligns well with the PR objective to prevent crash loops during slow startup. However, consider the following recommendations:
- Stagger the probe timings - typically, the readiness probe should start before the liveness probe to ensure the pod is ready to serve traffic before being checked for liveness.
- The readiness probe is using the same path as the liveness probe ('/liveness'). Consider using a dedicated readiness endpoint that checks application readiness.
Consider applying this configuration:
liveness: - initialDelaySeconds: 20 + initialDelaySeconds: 30 path: '/liveness' timeoutSeconds: 3 readiness: initialDelaySeconds: 20 - path: '/liveness' + path: '/readiness' timeoutSeconds: 3This change would:
- Start readiness checks at 20s to begin accepting traffic when ready
- Start liveness checks at 30s to allow more time for full initialization
- Use a dedicated readiness endpoint for proper readiness checking
Also applies to: 149-150
charts/services/application-system-api/values.staging.yaml (1)
145-146: Health check delay configuration needs refinementWhile increasing the initial delay to 20 seconds aligns with the PR objective to prevent crash loops, setting identical delays for both liveness and readiness probes is not the optimal approach. The readiness probe should typically start before the liveness probe.
Consider the following probe timing strategy:
- Keep liveness probe's
initialDelaySeconds: 20to prevent premature restarts- Reduce readiness probe's
initialDelaySecondsto 10 seconds to allow faster pod scheduling- Add
periodSecondsandfailureThresholdto both probes for more granular controlliveness: initialDelaySeconds: 20 path: '/liveness' timeoutSeconds: 3 + periodSeconds: 10 + failureThreshold: 3 readiness: - initialDelaySeconds: 20 + initialDelaySeconds: 10 path: '/liveness' timeoutSeconds: 3 + periodSeconds: 5 + failureThreshold: 3Rationale:
- Readiness probe starting earlier helps Kubernetes make informed scheduling decisions
- Liveness probe starting later prevents unnecessary restarts during slow startups
- Added parameters provide better control over probe behavior
Also applies to: 149-150
charts/islandis/values.staging.yaml (1)
699-704: Consider standardizing startup delays across related servicesOther services in the cluster that share similar startup characteristics might benefit from consistent initialization delays. Consider reviewing and adjusting the following services:
- application-system-api-worker
- application-system-form
charts/islandis/values.dev.yaml (3)
703-707: Consider using different paths for liveness and readiness probesWhile the 20-second initial delay is appropriate for preventing crash loops during startup, both probes are using the same path '/liveness'. Consider using separate endpoints:
- Liveness probe: Keep '/liveness' to check if the application is alive
- Readiness probe: Use a different endpoint (e.g., '/readiness') to verify the application is ready to serve traffic
Line range hint
2780-2784: Align liveness probe delay with readiness probeThe readiness probe has a 20-second delay but the liveness probe only has 3 seconds. This could cause the liveness probe to fail while the readiness probe is still waiting. Consider setting both to 20 seconds for consistency with the application-system-api configuration.
liveness: - initialDelaySeconds: 3 + initialDelaySeconds: 20 path: '/liveness' timeoutSeconds: 3
Increase startup probe delays for database-dependent services
The search reveals that most services use a 3-second delay, while application-system-api uses 20 seconds. Other database-dependent services should also have increased delays to prevent premature restarts during database initialization:
- application-system-api (already at 20s)
- service-portal-api
- services-documents
- services-sessions
- icelandic-names-registry-backend
- regulations-admin-backend
🔗 Analysis chain
Line range hint
1-3000: Review startup times for other servicesWhile this PR addresses the startup delay for application-system-api, other services might benefit from similar adjustments. Consider:
- Reviewing startup times of dependent services
- Applying consistent probe delays across related services
- Documenting the rationale for different delay values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all services with short initial delays that might need adjustment echo "Services with short initial delays (< 10 seconds):" grep -B 2 -A 2 "initialDelaySeconds: [1-9]" . | grep -B 2 "path: '/liveness'"Length of output: 220
Script:
#!/bin/bash # Search for health check configurations with initial delays rg -A 5 "healthCheck:" | rg -A 4 "liveness:" | rg -A 2 "initialDelaySeconds:"Length of output: 66725
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
apps/application-system/api/infra/application-system-api.ts(1 hunks)charts/islandis/values.dev.yaml(1 hunks)charts/islandis/values.prod.yaml(1 hunks)charts/islandis/values.staging.yaml(1 hunks)charts/services/application-system-api/values.dev.yaml(1 hunks)charts/services/application-system-api/values.prod.yaml(1 hunks)charts/services/application-system-api/values.staging.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/application-system/api/infra/application-system-api.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (7)
charts/services/application-system-api/values.prod.yaml (1)
145-145: The increase in initial delay looks good!
The change to set initialDelaySeconds: 20 for both probes will help mitigate crash loops during slow startups, which aligns well with the PR objective.
Also applies to: 149-149
charts/services/application-system-api/values.staging.yaml (1)
146-146: Verify the health check endpoint configuration
Both liveness and readiness probes are using the same endpoint '/liveness'. This might not be the intended configuration as readiness typically requires different checks than liveness.
Let's verify if separate endpoints exist for these probes:
Also applies to: 150-150
charts/islandis/values.staging.yaml (2)
699-704: Health check configuration changes look good
The increase of initialDelaySeconds from 3 to 20 seconds for both liveness and readiness probes aligns with the PR objective to prevent crash loops during slow application startup. This change gives the application more time to properly initialize before health checks begin.
699-704: Verify startup time metrics
To ensure the 20-second delay is appropriate, it would be valuable to verify actual startup times.
✅ Verification successful
Based on the analysis of the codebase, I can provide a response regarding the 20-second delay configuration:
20-second delay appears appropriate for application-system-api
The 20-second initial delay is specifically configured for the application-system-api service and is consistently set across all environments (dev, staging, prod). This longer delay appears intentional and appropriate because:
- The service requires database connectivity and migrations
- Most other services use a 3-second delay, indicating this service has unique startup requirements
- The same delay is used for both liveness and readiness probes, suggesting thorough startup validation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check pod startup times and potential crash loops in recent history
# This will help validate if 20 seconds is an appropriate delay
# Get pod events for the application-system-api namespace
kubectl get events -n application-system --sort-by='.lastTimestamp' | grep -i "application-system-api"
# Check pod restart history
kubectl get pods -n application-system -l app=application-system-api --sort-by=.status.startTime
Length of output: 349
Script:
#!/bin/bash
# Let's analyze the repository for any startup time metrics, logs, or tests
# that might indicate application startup behavior
# Search for startup time related configurations
rg -i "startup|boot|initial.*delay" -A 3 -B 3
# Look for any test files that might contain startup timing information
fd -e test.js -e spec.js -e test.ts -e spec.ts --exec rg -i "startup|boot|timing"
# Search for any documentation about startup requirements
fd README.md --exec cat {} \; | rg -i "startup|boot|initial.*delay" -A 3 -B 3
# Look for any health check implementations
ast-grep --pattern 'app.get("/liveness"'
Length of output: 299903
charts/islandis/values.prod.yaml (1)
690-694: LGTM! The increased initial delay should help prevent crash loops.
The change from 3 to 20 seconds for both liveness and readiness probe initial delays is appropriate for preventing premature failure detection during slow application startups.
Let's check if other services might benefit from similar adjustments:
apps/application-system/api/infra/application-system-api.ts (2)
349-350: Liveness and Readiness Probe Configuration Updated Appropriately
The liveness and readiness probes have been updated with an initialDelaySeconds of 20, which should help mitigate crash loops due to slow startup times.
Line range hint 356-360: Verify the Maximum Replica Count Matches Scaling Requirements
The max value in the .replicaCount configuration is set to 60. Please verify if this should be increased to 70 to align with the intended scaling policies, as mentioned in the PR objectives and AI-generated summary.
Run the following script to check for consistency of replicaCount configurations:
norda-gunni
added
autoupdate
* chore(application-system): Set initialDelay to 20 seconds * chore: charts update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* chore(application-system): Set initialDelay to 20 seconds (#16893) * chore(application-system): Set initialDelay to 20 seconds * chore: charts update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(application-system): Increase cpu and mem limit and request to avoid crashloops (#16792) * chore(application-system): Increase cpu and mem limit and request to avoid crashloops * chore: charts update dirty files * Decrease cpu resources and return memory to original value * chore: charts update dirty files * chore: charts update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * Remove charts --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* chore(application-system): Set initialDelay to 20 seconds * chore: charts update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
...
Attach a link to issue if relevant
What
Add initial delay to the liveness probe
Why
Mitigate crashloops due to slow startup
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores