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

refactor(stopFlag): completly remove stopFlag for context #440

Merged
merged 5 commits into from
Dec 11, 2024

Conversation

CodeLieutenant
Copy link
Contributor

context is Go package used mainly to signal cancelation.
stopFlag package inside of gemini was used the same way.
It was just adding overhead and maintainability issues in the long
run, as it was only a wrapper for context.Context and all at once
calling the cancel funcs at the end of the program. This removal
makes code easier to follow for everybody working on it. context
package is well known in Go community and can serve many purposes
beyond just canceling the background workers (jobs in gemini
terminology).

Hard kill in gemini was used to signal the immadiet stoppage
for gemini, without pritting the results, and soft on the contrary
could be triggers in a few ways:

  • gemini validation failed
  • gemini mutation failed
  • SIG(INT,TERM) was sent

There is no reason to have HARD kill in the application, if we need hard
kill, SIGKILL can be sent and everything would be stopped, there will be
no need to the the cleanup as kernel will ensure that happens.

context.Context works as a soft kill, and if something happens bad
in gemini (validation fails or mutation fail) globalStatus.HasError()
would stop all other goroutines from continuing if failFast CLI flag
is passed, so the set of softKill is not needed.

@CodeLieutenant CodeLieutenant self-assigned this Nov 28, 2024
@CodeLieutenant CodeLieutenant added the enhancement New feature or request label Nov 28, 2024
@CodeLieutenant CodeLieutenant added this to the Gemini 1.9 milestone Nov 28, 2024
@CodeLieutenant CodeLieutenant linked an issue Dec 2, 2024 that may be closed by this pull request
In process of removing the `stopFlag` from gemini's codebase,
first step is to migrate the `Value Generators` for patitions.
Using context with generators make a lot more sense then the custom
built, `stopFlag`. `context` is built-in package in Go, and
this is it's usecase - cancelation propagation to background task.

Signed-off-by: Dusan Malusev <[email protected]>
In process of removing the `stopFlag` from gemini's codebase,
first step is to migrate the `Value Generators` for patitions.
Using context with generators make a lot more sense then the custom
built, `stopFlag`. `context` is built-in package in Go, and
this is it's usecase - cancelation propagation to background task.

Signed-off-by: Dusan Malusev <[email protected]>
`context` is Go package used mainly to signal cancelation.
`stopFlag` package inside of gemini was used the same way.
It was just adding overhead and maintainability issues in the long
run, as it was only a wrapper for `context.Context` and all at once
calling the `cancel` funcs at the end of the program. This removal
makes code easier to follow for everybody working on it. `context`
package is well known in Go community and can serve many purposes
beyond just canceling the background workers (`jobs` in gemini
terminology).

Hard kill in gemini was used to signal the immadiet stoppage
for gemini, without pritting the results, and soft on the contrary
could be triggers in a few ways:

- gemini validation failed
- gemini mutation failed
- SIG(INT,TERM) was sent

There is no reason to have HARD kill in the application, if we need hard
kill, SIGKILL can be sent and everything would be stopped, there will be
no need to the the cleanup as kernel will ensure that happens.

`context.Context` works as a soft kill, and if something happens bad
in gemini (validation fails or mutation fail) `globalStatus.HasError()`
would stop all other `goroutines` from continuing if `failFast` CLI flag
is passed, so the set of `softKill` is not needed.

Signed-off-by: Dusan Malusev <[email protected]>
@CodeLieutenant CodeLieutenant marked this pull request as ready for review December 11, 2024 22:10
@CodeLieutenant CodeLieutenant merged commit 01d50a9 into scylladb:master Dec 11, 2024
1 of 4 checks passed
@fruch
Copy link
Collaborator

fruch commented Dec 12, 2024

@CodeLieutenant what's the haste to get this merged ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stopFlag sometimes does not stop
2 participants