Skip to content

Commit 3dfc78e

Browse files
michaelkedartymzd
authored andcommitted
fix(recordchecker): potential precision issues + better logs (google#5135)
According to docs, [GCS Custom-Time is millisecond precision](https://docs.cloud.google.com/storage/docs/metadata#custom-time), [Datastore is microsecond](https://docs.cloud.google.com/datastore/docs/reference/data/rest/Shared.Types/Value#:~:text=A%20timestamp%20value.%20When%20stored%20in%20the%20Datastore%2C%20precise%20only%20to%20microseconds%3B%20any%20additional%20precision%20is%20rounded%20down.) (though Datastore now uses Firestore which is apparantly nanosecond precision 🤷). I'm wondering if this is causing issues with the recordchecker thinking things are out of date, but I would expect this to loop infinitely if that were the case (unless something else writing is millisecond-precise?) Added millisecond leeway and some better logging for this.
1 parent 62f5012 commit 3dfc78e

1 file changed

Lines changed: 11 additions & 2 deletions

File tree

go/cmd/recordchecker/recordchecker.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,15 +293,24 @@ func checkRecord(ctx context.Context, cl *datastore.Client, storageCl clients.Cl
293293
attrs, err := storageCl.ReadObjectAttrs(ctx, fmt.Sprintf("all/pb/%s.pb", id))
294294
if err != nil {
295295
res.needsRetry = true
296-
if !errors.Is(err, clients.ErrNotFound) {
296+
if errors.Is(err, clients.ErrNotFound) {
297+
logger.InfoContext(ctx, "record not found in gcs", slog.String("id", id))
298+
} else {
297299
// Log the error if it's not the expected "not found" error.
298300
res.err = fmt.Errorf("failed to read object for %s: %w", id, err)
299301
}
300302

301303
return res
302304
}
303305

304-
if attrs.CustomTime.Before(vuln.Modified) {
306+
// CustomTime is millisecond precision, while Modified is microsecond precision.
307+
// So we add a millisecond to CustomTime to account for the difference.
308+
if attrs.CustomTime.Add(time.Millisecond).Before(vuln.Modified) {
309+
logger.InfoContext(ctx, "record is stale in gcs",
310+
slog.String("id", id),
311+
slog.String("custom_time", attrs.CustomTime.String()),
312+
slog.String("modified", vuln.Modified.String()),
313+
)
305314
res.needsRetry = true
306315
}
307316

0 commit comments

Comments
 (0)