Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

[CBR-211] log rotation checks for size and age of files #3507

Merged
merged 1 commit into from
Aug 30, 2018

Conversation

CodiePP
Copy link
Contributor

@CodiePP CodiePP commented Aug 29, 2018

Description

log rotation for 'katip'

  • log files are named with "iso" date appended to filename: logs/node.log-20180829201517
  • configuration (--log-config) defines rotation parameters:
    ** write to new file if max size is reached (currently 5 MB)
    ** write to new file if current log file was created more than ''t'' (default: 24) hours ago
    ** remove oldest files and only keep ''n'' files (currently 20)
    (since IO is quite expensive, the removal of files from the filesystem is only done at most every 10 seconds)

Linked issue

CBR-211
CBR-97 (user story)

Type of change

  • [~] 🐞 Bug fix (non-breaking change which fixes an issue)
  • 🛠 New feature (non-breaking change which adds functionality)
  • [~] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • [~] 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
  • [~] 🔨 New or improved tests for existing code
  • [~] ⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

  • I have read the style guide document, and my code follows the code style of this project.
  • If my code deals with exceptions, it follows the guidelines.
  • I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
  • [~] CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

  • [~] I have added tests to cover my changes.
  • All new and existing tests passed.

QA Steps

the file util/Pos/Util/Log/Rotator.hs contains some commands at the end which can be entered in GHCi for testing.

$ stack ghci util/Pos/Util/Log.hs

@CodiePP CodiePP self-assigned this Aug 29, 2018
@CodiePP CodiePP requested review from erikd and njd42 August 29, 2018 20:44
scribeSettings :: KC.ScribeSettings
scribeSettings = KC.ScribeSettings bufferSize
where
bufferSize = 5000 -- size of the queue (in log items)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can set the size of katip's queue here.

let le1 = updateEnv le0 getCurrentTime
-- use 'getCurrentTime' to get a more precise timestamp
-- as katip uses per default some internal buffered time variable
timer <- mkAutoUpdate defaultUpdateSettings { updateAction = getCurrentTime, updateFreq = 10000 }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the time is fetched from the OS at most 100 times a second.

mkFileDescription bp fp =
-- if fp contains a filename in a directory path
-- move this path to the prefix and only keep the name
let (extbp, fname) = splitFileName fp
Copy link
Contributor Author

Choose a reason for hiding this comment

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

some log-config files contain as filename a path like this: pub/node.pub
in that case, we add the directory path to the prefix dir and keep the filename apart.

trp <- initializeRotator rot fdesc
scribestate <- newMVar trp -- triple of (handle), (bytes remaining), (rotate time)
-- sporadically remove old log files - every 10 seconds
cleanup <- mkAutoUpdate defaultUpdateSettings { updateAction = cleanupRotator rot fdesc, updateFreq = 10000000 }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleanup of old files which outnumber the limit set in the configuration file are deleted from the filestystem, at most every 10 seconds. (updateFreq is actually the sleep time).

bracket_ (takeMVar locklocal) (putMVar locklocal ()) $
T.hPutStrLn h $ encodeToLazyText $ itemJson v item
pure $ Scribe logger (hFlush h)
modifyMVar_ scribestate $ \(hdl, bytes, rottime) -> do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this part is based on the ingenious proposal by Neil. Thanks!

mkFileScribeH :: Handle -> Bool -> Severity -> Verbosity -> IO Scribe
mkFileScribeH h colorize s v = do
hSetBuffering h LineBuffering
locklocal <- newMVar ()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only local lock

-- | global lock for file Scribes
{-# NOINLINE lock #-}
lock :: MVar ()
lock = unsafePerformIO $ newMVar ()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no more global lock.

@CodiePP CodiePP force-pushed the adiemand/CBR-211/log-rotation branch 2 times, most recently from c55269a to e0fc654 Compare August 29, 2018 21:02
@erikd
Copy link
Member

erikd commented Aug 29, 2018

The build failures here are due to a bad Guthub merge which has now been fixed. If you rebase against develop and then force push, it should build.

@erikd erikd force-pushed the adiemand/CBR-211/log-rotation branch from e0fc654 to f15d462 Compare August 29, 2018 22:13
Copy link
Member

@erikd erikd left a comment

Choose a reason for hiding this comment

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

LGTM!

@erikd
Copy link
Member

erikd commented Aug 29, 2018

All AppVeyor builds are failing because a file download is failing.

@erikd erikd merged commit 5659d4f into develop Aug 30, 2018
@erikd erikd deleted the adiemand/CBR-211/log-rotation branch August 30, 2018 03:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants