journal: make current log file have a fixed name #7112
Conversation
There was a problem hiding this comment.
So this will lead to slightly different semantics on the naming of journal files. Right now we stamp journals with the timestamp at which they were opened. This will stamp them with the date at which they were closed/rotated.
I'd honestly prefer to have a date range in there, even if it results in an excessively long filename.
Also to consider is the behaviour on restart -- this will stamp the existing journal with the timestamp of the restart. Currently we start a new journal with the date of the restart, which I think it's more intuitive.
journal/fs.go
Outdated
| nfi, err := os.Create(filepath.Join(f.dir, fmt.Sprintf("lotus-journal-%s.ndjson", build.Clock.Now().Format(RFC3339nocolon)))) | ||
| // check if journal file exists | ||
| if _, err := os.Stat(filepath.Join(f.dir, "lotus-journal.json")); err == nil { | ||
| err := os.Rename(filepath.Join(f.dir, "lotus-journal.json"), filepath.Join(f.dir, fmt.Sprintf("lotus-journal-%s.ndjson", build.Clock.Now().Format(RFC3339nocolon)))) |
There was a problem hiding this comment.
nit: that's a long line, can we break it out in local vars?
journal/fs.go
Outdated
| @@ -109,9 +109,17 @@ func (f *fsJournal) rollJournalFile() error { | |||
| _ = f.fi.Close() | |||
| } | |||
|
|
|||
| nfi, err := os.Create(filepath.Join(f.dir, fmt.Sprintf("lotus-journal-%s.ndjson", build.Clock.Now().Format(RFC3339nocolon)))) | |||
| // check if journal file exists | |||
| if _, err := os.Stat(filepath.Join(f.dir, "lotus-journal.json")); err == nil { | |||
There was a problem hiding this comment.
Technically, this could be a directory, but even if it is we want to rename it, so it's fine that we don't use the stat output.
|
@raulk All these comments should be resolved. I also realised that I was using |
journal/fs.go
Outdated
| current := filepath.Join(f.dir, "lotus-journal.ndjson") | ||
| rolled := filepath.Join(f.dir, fmt.Sprintf( | ||
| "lotus-journal-%s-%s.ndjson", | ||
| f.start.Format(RFC3339nocolon), end.Format(RFC3339nocolon), |
There was a problem hiding this comment.
This breaks when we restart, because f.start will no longer have the original start timestamp, so the stamping would be inaccurate. We could stamp the file with the start timestamp, and then append the end timestamp on rotation, but that would make the current filename dynamic again. I'm OK just accepting the semantic change to stamp the rotated file with the end date.
journal/fs.go
Outdated
|
|
||
| nfi, err := os.Create(filepath.Join(f.dir, fmt.Sprintf("lotus-journal-%s.ndjson", build.Clock.Now().Format(RFC3339nocolon)))) | ||
| nfi, err := os.Create(filepath.Join(f.dir, "lotus-journal.json")) |
There was a problem hiding this comment.
| nfi, err := os.Create(filepath.Join(f.dir, "lotus-journal.json")) | |
| nfi, err := os.Create(filepath.Join(f.dir, "lotus-journal.ndjson")) |
|
@lanzafame Apologies for back and forth, but I can't imagine a simple way of remembering the original start date on restart. I'm happy to accept the original semantic change where the timestamp is now the end/rotation timestamp. This is also how I believe frameworks like log4j handle rotation. Also, caught a small bug. |
raulk
changed the title
|
@lanzafame Thank you! |
raulk
changed the title
Resolves #7050.