Skip to content

Conversation

@AdheipSingh
Copy link
Contributor

@AdheipSingh AdheipSingh commented Aug 29, 2024

  • Handling of SIGTERM. This is needed for readiness probe to work, so whenever kubelet sends a SIGTERM to pod, readiness handler response unavailable, this way service will send no traffic to this pod.

  • This PR also makes sure actix drains HTTP connection before exiting.

how to stop traffic. The current issue is service is still sending traffic to terminating pods. We added pre-stop hooks to sleep for kube proxy and core dns cache to clear, which worked at times, but failed too. So now, we want to make sure connections are drained.
Handling SIGTERM
1. Kubelet sends SIGTERM to pod when we issue delete pod
2. Actix hijacks the call ( thread is running ) and tweaks a global flag SIGNAL_RECEIVED to true.
3.Readiness Probe handler reads the flag each time, it is called.
4. on pod readiness probe is set, kubelet will call probe every 5 seconds.
5. Readiness Probe will return unavailable, and pod will go 0/1 unavailable.
6. Service at this point will route to other available pods.
7. After 120 seconds pod will terminate, during that period ingestor will s3 sync ( TODO: once this is working on k8s i will add in SIGTERM handling itself )

Description


This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

@AdheipSingh AdheipSingh marked this pull request as draft August 29, 2024 18:44
@AdheipSingh AdheipSingh changed the title [WIP] Handle SIGTERM for Distributed and Standalone Mode Handle SIGTERM for Distributed and Standalone Mode Sep 1, 2024
@AdheipSingh AdheipSingh marked this pull request as ready for review September 1, 2024 17:16
@AdheipSingh
Copy link
Contributor Author

  • I need to build the image from Dockerfile and test in cluster once again. I have been testing using Dockerfile.debug image.
    This will also need changes in existing helm charts. To add readiness probe.
    Please suggest if you want to add in same PR or new one.
    cc @nitisht @nikhilsinhaparseable

@nitisht
Copy link
Member

nitisht commented Sep 1, 2024

Please suggest if you want to add in same PR or new one.

Let's add in the same PR

Dockerfile Outdated
WORKDIR /parseable

# Copy the static shell into base image.
COPY --from=builder /bin/sh /bin/sh
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nitisht any specific reason for adding shell. Is it because image is distroless, i have been using Dockerfile.debug, so just want to confirm ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because distroless doesn't have any other shell.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but do we need a shell ? I mean wasn't shell for ${hostname}, container and app PID should be same.
I'lll check once if needed. If you see CMD will trigger the command itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can remove in that case

Dockerfile Outdated
WORKDIR /parseable

# Copy the static shell into base image.
COPY --from=builder /bin/sh /bin/sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because distroless doesn't have any other shell.

@AdheipSingh
Copy link
Contributor Author

Please suggest if you want to add in same PR or new one.

Let's add in the same PR

Done

@AdheipSingh
Copy link
Contributor Author

TODO: to be tested with Dockerfile changes in cluster.

@AdheipSingh AdheipSingh changed the title Handle SIGTERM for Distributed and Standalone Mode Handling SIGTERM for Distributed and Standalone Mode Sep 1, 2024
@AdheipSingh
Copy link
Contributor Author

Dockefile changes are testing. No shell copy needed.

@AdheipSingh AdheipSingh requested a review from nitisht September 3, 2024 16:18
@nitisht nitisht merged commit 7ab2dd4 into parseablehq:main Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants