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

Problem: No log rotation when BigchainDB and Tendermint started with Monit #2528

Merged
merged 7 commits into from Sep 12, 2018

Conversation

muawiakh
Copy link
Contributor

@muawiakh muawiakh commented Sep 10, 2018

Solution

Currently, BigchainDB server, takes care of log rotation using logging module.
This enforces log rotation if the file size exceeds 209715200 i.e. for bigchaindb-benchmark.log, bigchaindb.log, bigchaindb-errors.log.

  • Remove the extra bigchaindb.err.log and bigchaindb.out.log redirection, because BigchainDB also stores logs and has log rotation for the relevant files instead redirect to /dev/null
  • Introduce logrotate script.
  • Fix a minor log message.
  • Add tendermint log rotation for tendermint.err.log and tendermint.out.log if the size exceeds 200 MB.
  • Add docs for log rotation

@codecov-io
Copy link

Codecov Report

Merging #2528 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2528   +/-   ##
=======================================
  Coverage   93.53%   93.53%           
=======================================
  Files          42       42           
  Lines        2551     2551           
=======================================
  Hits         2386     2386           
  Misses        165      165

@@ -110,6 +110,7 @@ case \$1 in
start_tendermint)

pushd \$4

nohup tendermint node --consensus.create_empty_blocks=false >> \$3/tendermint.out.log 2>> \$3/tendermint.err.log &
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we need the --consensus.create_empty_blocks=false in the command to start Tendermint, because that's ensured by the settings we tell them to put in config.toml, i.e. see http://docs.bigchaindb.com/projects/server/en/master/simple-deployment-template/network-setup.html#member-connect-to-the-other-members

@@ -93,7 +93,7 @@ case \$1 in
start_bigchaindb)

pushd \$4
nohup bigchaindb -l DEBUG start >> \$3/bigchaindb.out.log 2>> \$3/bigchaindb.err.log &
nohup bigchaindb -l DEBUG start > /dev/null 2>&1 &
Copy link
Contributor

Choose a reason for hiding this comment

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

Why set logging level to debug? I think its better to leave logging flag out of the command here and let it pick up logging configs from the config file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a test network, but I can change if that is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Some people might use our Monit script for their production node.

@@ -161,9 +178,17 @@ check process bigchaindb
check process tendermint
with pidfile ${monit_pid_path}/tendermint.pid
start program "${monit_script_path} start_tendermint ${monit_pid_path}/tendermint.pid ${monit_log_path} ${monit_log_path}"
restart program "${monit_script_path} start_bigchaindb ${monit_pid_path}/bigchaindb.pid ${monit_log_path} ${monit_log_path}"
restart program "${monit_script_path} start_tendermint ${monit_pid_path}/tendermint.pid ${monit_log_path} ${monit_log_path}"
Copy link
Contributor

@shahbazn shahbazn Sep 10, 2018

Choose a reason for hiding this comment

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

does restart here stops and then starts again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessarily, restart just happens when the service is not running.

@@ -0,0 +1,46 @@
Copyright BigchainDB GmbH and BigchainDB contributors
Copy link
Contributor

Choose a reason for hiding this comment

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

The block at the top should be an HTML comment (so it doesn't render on ReadTheDocs). To do that, you will have to add <!-- or <!--- before it, maybe as a new line 1. (All HTML markup is valid Markdown markup.)


Default location for these log files is: `$HOME/.bigchaindb-monit/logs`

The logs for BigchainDB server are rotated when any of the above mentioned file exceeds `209715200 bytes`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why that number of bytes. I guess that is the default inherited from the logging module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its already part of the BigchainDB code. https://github.com/bigchaindb/bigchaindb/blob/master/bigchaindb/log.py#L49

Not entirely sure why.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe write "209715200 bytes (i.e. approximately 209 MB)." since 209 MB is easier to understand for humans.

/bin/mv \$2 \$2.\$(date +%y-%m-%d)
/bin/tar -cvf \$2.\$(date +%Y%m%d_%H%M%S).tar.gz \$2.\$(date +%y-%m-%d)
/bin/rm \$2.\$(date +%y-%m-%d)
/bin/kill -2 \$(cat \$3)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, you can clear the file without removing it, then Tendermint does not have to be restarted. One way of clearing it is cp /dev/null \$2.


## Log rotation for BigchainDB

Log rotation is baked into BigchainDB server using the `logging` module. If you followed the [How to Set Up a BigchainDB Network](../simple-deployment-template/network-setup.md) guide. BigchainDB processes are monitored using [Monit]( https://www.mmonit.com/monit). BigchainDB server logs information into the following files:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mention Monit in the BigchainDB section?

Also, makes sense to add that bigchaindb.log and other log files are by default created in the directory from where bigchaindb is run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rephrased the sentence, i think it makes sense now for a monit based user as well as a normal user.


## Log rotation for Tendermint

Log rotation for Tendermint is handled by [Monit]( https://www.mmonit.com/monit). When we start BigchainDB and Tendermint using [How to Set Up a BigchainDB Network](../simple-deployment-template/network-setup.md) guide.
Copy link
Contributor

Choose a reason for hiding this comment

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

Log rotation for Tendermint is handled by Monit.

It kinda suggests that Monit is always there. I would say something like "you need to use Monit setup to use the log rotation scripts prepared by us".

Code is Apache-2.0 and docs are CC-BY-4.0
--->

# Log rotation for a BigchainDB node
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a new docs page so it must be added to an index.rst file otherwise Sphinx will say, for example:

Warning, treated as error:
/home/troy/repos/bigchaindb/docs/server/source/appendices/log-rotation.md:document isn't included in any toctree

It's in the appendices directory, so that's the index.rst file that should be updated, i.e.

docs/server/source/appendices/index.rst

Copy link
Contributor Author

Choose a reason for hiding this comment

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

forgot to add it will add in next commit.

- Rephrase log rotation to imply monit is not default
- Add statement to explain where bigchaindb log files are created
- Fix HTML comment
- Add log rotation to index
@ttmc ttmc merged commit 5a44084 into bigchaindb:master Sep 12, 2018
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.

None yet

5 participants