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.

Sign up for GitHub

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

chore!: docker log to stdout and minor logging tweaks #951

Merged
merged 6 commits into from
Nov 4, 2024
Merged

chore!: docker log to stdout and minor logging tweaks #951

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e3cd87f
chore: error on catchup replay is just a warning
lklimek Oct 10, 2024
9dda3b6
chore(docker): redirect stderr to stdout
lklimek Oct 10, 2024
555208a
Merge branch 'v1.3-dev' into chore-logs-stdout
lklimek Oct 11, 2024
ac2bc10
Merge branch 'v1.3-dev' into chore-logs-stdout
lklimek Nov 2, 2024
fa21dc9
Merge branch 'v1.3-dev' into chore-logs-stdout
lklimek Nov 2, 2024
8a05a4b
Merge branch 'v1.3-dev' into chore-logs-stdout
lklimek Nov 4, 2024
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
13 changes: 6 additions & 7 deletions DOCKER/docker-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#!/bin/bash


# TODO: Workaround for busy port problem happening with docker restart policy
# we are trying to start a new container but previous process still not release
# the port.
Expand All @@ -15,9 +14,9 @@ got_signal=false
# Function to handle signals and forward them to the tenderdash process
# shellcheck disable=SC2317
_forward_signal() {
echo "Caught signal! Forwarding to tenderdash process."
got_signal=true
kill -s "$1" "$child"
echo "Caught signal! Forwarding to tenderdash process."
got_signal=true
kill -s "$1" "$child"
}

# Trap signals and forward them to the tenderdash process
Expand Down Expand Up @@ -52,14 +51,14 @@ if [ ! -d "$TMHOME/config" ]; then
fi

# Start tenderdash in the background
tenderdash "$@" &
tenderdash "$@" 2>&1 &
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you want to send all stderr from Tenderdash? We should probably let Tenderdash control output and adjust the logger to use stdout instead of stderr. There might be some confusion in the future when you do some stderr output from Tenderdash and do not see it when you run a docker container.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My AI junior says:

For server software running on Linux, it is generally recommended to send logs to `stderr`. This is because `stdout` is typically used for regular output, while `stderr` is used for logging errors and diagnostic information. Sending logs to `stderr` helps to separate log messages from the normal output of the program, making it easier to manage and analyze logs.

Here's a summary of the best practices:

- **stdout**: Use for regular program output, such as the results of a command or data that the program is designed to produce.
- **stderr**: Use for logging errors, warnings, and diagnostic information.

By following this convention, you can ensure that logs are properly handled and can be redirected or processed separately from the main output of your server software.

So, Tenderdash behavior to send logs to stderr is correct. For a particular use case of Docker, I decided to join these streams for usability.

child=$!
wait "$child"
exit_code=$?

if [ $got_signal == false ] && [ $exit_code -ne 0 ] && [ "$1" == "start" ]; then
echo "Sleeping for 10 seconds as workaround for the busy port problem. See entrypoint code for details."
sleep 10
echo "Sleeping for 10 seconds as workaround for the busy port problem. See entrypoint code for details."
sleep 10
fi

exit $exit_code
2 changes: 1 addition & 1 deletion internal/consensus/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ func (cs *State) OnStart(ctx context.Context) error {
break LOOP

case !IsDataCorruptionError(err):
cs.logger.Error("error on catchup replay; proceeding to start state anyway", "err", err)
cs.logger.Warn("error on catchup replay; proceeding to start state anyway", "err", err)
break LOOP

case repairAttempted:
Expand Down
Loading