Skip to content

Commit dd58629

Browse files
chressiestapelberg
authored andcommitted
glog: Don't try to create/rotate a given syncBuffer twice in the same second
If you do, then you truncate the existing file. So logging too much too quickly would lose log data. cl/709080575 (google-internal)
1 parent 04dbec0 commit dd58629

File tree

2 files changed

+61
-14
lines changed

2 files changed

+61
-14
lines changed

glog_file.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -248,16 +248,22 @@ type syncBuffer struct {
248248
names []string
249249
sev logsink.Severity
250250
nbytes uint64 // The number of bytes written to this file
251+
madeAt time.Time
251252
}
252253

253254
func (sb *syncBuffer) Sync() error {
254255
return sb.file.Sync()
255256
}
256257

257258
func (sb *syncBuffer) Write(p []byte) (n int, err error) {
259+
// Rotate the file if it is too large, but ensure we only do so,
260+
// if rotate doesn't create a conflicting filename.
258261
if sb.nbytes+uint64(len(p)) >= MaxSize {
259-
if err := sb.rotateFile(time.Now()); err != nil {
260-
return 0, err
262+
now := timeNow()
263+
if now.After(sb.madeAt.Add(1*time.Second)) || now.Second() != sb.madeAt.Second() {
264+
if err := sb.rotateFile(now); err != nil {
265+
return 0, err
266+
}
261267
}
262268
}
263269
n, err = sb.Writer.Write(p)
@@ -276,6 +282,7 @@ func (sb *syncBuffer) rotateFile(now time.Time) error {
276282
var err error
277283
pn := "<none>"
278284
file, name, err := create(sb.sev.String(), now)
285+
sb.madeAt = now
279286

280287
if sb.file != nil {
281288
// The current log file becomes the previous log at the end of

glog_test.go

+52-12
Original file line numberDiff line numberDiff line change
@@ -521,21 +521,24 @@ func logAtVariousLevels() {
521521

522522
func TestRollover(t *testing.T) {
523523
setFlags()
524+
defer func(previous func() time.Time) { timeNow = previous }(timeNow)
525+
526+
// Initialize a fake clock that can be advanced with the tick func.
527+
fakeNow := time.Date(2024, 12, 23, 1, 23, 45, 0, time.Local)
528+
timeNow = func() time.Time {
529+
return fakeNow
530+
}
531+
532+
tick := func(d time.Duration) {
533+
fakeNow = fakeNow.Add(d)
534+
}
524535

525536
Info("x") // Be sure we have a file.
526537
info, ok := sinks.file.file[logsink.Info].(*syncBuffer)
527538
if !ok {
528539
t.Fatal("info wasn't created")
529540
}
530541

531-
// Make sure the next log file gets a file name with a different
532-
// time stamp.
533-
//
534-
// TODO: determine whether we need to support subsecond log
535-
// rotation. C++ does not appear to handle this case (nor does it
536-
// handle Daylight Savings Time properly).
537-
time.Sleep(1 * time.Second)
538-
539542
// Measure the current size of the log file.
540543
info.Flush()
541544
fi, err := info.file.Stat()
@@ -550,7 +553,9 @@ func TestRollover(t *testing.T) {
550553

551554
fname0 := info.file.Name()
552555

553-
// Force a rotation.
556+
// Advance clock by 1.5 seconds to force rotation by size.
557+
// (The .5 will be important for the last test as well).
558+
tick(1500 * time.Millisecond)
554559
Info(longMessage)
555560
Info(longMessage)
556561
info.Flush()
@@ -610,14 +615,49 @@ func TestRollover(t *testing.T) {
610615

611616
// Make sure Names returned the right names.
612617
n, err := Names("INFO")
613-
if len(n) != 2 && err != nil && n[0] != fname0 && n[1] != fname1 {
618+
if (len(n) != 2 || err != nil) && n[0] != fname0 && n[1] != fname1 {
614619
t.Errorf("Names(INFO) wanted [%s, %s]/nil, got %v/%v", fname0, fname1, n, err)
615620
}
616621

622+
// The following tests assume that previous test left clock at .5 seconds.
623+
if fakeNow.Nanosecond() != 5e8 {
624+
t.Fatalf("BUG: fake clock should be exactly at .5 seconds")
625+
}
626+
627+
// Same second would create conflicting filename, no rotation expected.
628+
tick(499 * time.Millisecond)
629+
Info(longMessage)
630+
Info(longMessage)
631+
n, err = Names("INFO")
632+
if got, want := len(n), 2; got != want || err != nil {
633+
t.Errorf("Names(INFO) = %v (len=%v), %v, want %d names: expected no rotation within same second", n, got, err, want)
634+
}
635+
636+
// Trigger a subsecond rotation in next fakeClock second.
637+
tick(1 * time.Millisecond)
638+
Info(longMessage)
639+
Info(longMessage)
640+
n, err = Names("INFO")
641+
if got, want := len(n), 3; got != want || err != nil {
642+
t.Errorf("Names(INFO) = %v (len=%v), %v, want %d names: expected a rotation after under a second when filename does not conflict", n, got, err, want)
643+
}
644+
645+
// Trigger a rotation within a minute since the last rotation.
646+
tick(time.Minute)
647+
Info(longMessage)
648+
Info(longMessage)
649+
n, err = Names("INFO")
650+
if got, want := len(n), 4; got != want || err != nil {
651+
t.Errorf("Names(INFO) = %v (len=%v), %v, want %d names: expected a rotation after one minute since last rotation", n, got, err, want)
652+
}
653+
617654
if t.Failed() {
618-
t.Logf("%v:\n%q", fname0, f0)
619-
t.Logf("%v:\n%q", fname1, f1)
655+
t.Logf("========================================================")
656+
t.Logf("%s:\n%s", fname0, f0)
657+
t.Logf("========================================================")
658+
t.Logf("%s:\n%s", fname1, f1)
620659
}
660+
621661
}
622662

623663
func TestLogBacktraceAt(t *testing.T) {

0 commit comments

Comments
 (0)