Timestamped log names + per-entry log timezone#41
Merged
Conversation
Add float tz_offset_hours to KeyEntry — a GMT offset in hours (fractional allowed, e.g. 5.5 for IST, -3.75 for Chatham) that will drive both the date subdir and the log filename. It claims one of the reserved words (168-byte record size unchanged), so old records read back as 0.0 = GMT with no conversion. An offset is stored rather than a named zone deliberately: a fixed offset is unambiguous and DST-free, which is what a stable log-naming convention wants. New 'keydb.py settz PORT2 HOURS' CLI action, shown in list output as e.g. tz=GMT+05:30.
Replace the sessionN.tlog / sessionN.bin naming with a timestamp basename YYYY_MM_DD_HH:MM:SS of the session start, and form both the date subdir and the filename in the entry's log-naming timezone (tz_offset_hours, a GMT offset). session_time_strings() applies the offset then formats with gmtime so the machine's own TZ never leaks in; offset 0 is GMT. A single basename is computed once at fork (made unique across .tlog/.bin so a same-second start can't clobber) and shared by both writers, keeping the paired files together. binlog's reboot rotation recomputes a fresh reboot-time basename. next_session_n is gone; the writers now take (datedir, name).
Broaden the session-file regex to accept the new YYYY_MM_DD_HH:MM:SS[-N] timestamp names (keeping the legacy sessionN names so old logs stay browsable); the natural-sort key already orders the timestamps chronologically. Add a 'Log timezone (GMT offset in hours)' field to the owner and admin edit forms (validated -12..14), wired through both routes and rendered in both templates.
Update the binlog/tlog tests to glob the timestamp-named session files instead of hardcoded sessionN paths (filtering to the session-name pattern so seeded fixtures like prefill.bin don't get mistaken for a session, and sorting by mtime since the '-N' collision suffix breaks lexical order). Switch the date-dir helpers to UTC to match the proxy's GMT-default naming. Add coverage: a .bin named by the session timestamp in a +10h entry timezone; keydb tz round-trip, formatting, and settz CLI; webadmin listing/download of timestamp (and -N suffix) names; and saving the timezone via the owner edit form. Adjust the reserved-word count assertions (15 -> 14) for the slot the timezone field claimed.
Add KEY_FLAG_USE_TZ: when set, logs are named with the fixed tz_offset_hours GMT offset; when clear (the default, and what legacy records get) they are named in the server's own local timezone — so an existing install keeps its local-time naming with no migration, and 0 is a genuine GMT offset only when the flag is on. settz sets the offset and enables the flag; clear the flag to revert to local without losing the value. Validate the offset with math.isfinite() (NaN slips past plain range checks) and render a non-finite value as 'invalid'. (supersedes the earlier '0 = server local' approach; found by review.)
session_time_strings takes a use_offset flag: true applies the fixed GMT offset via gmtime (no DST), false formats in the server's local timezone. supportproxy derives it from KEY_FLAG_USE_TZ and threads it (plus the offset) to the tlog/binlog writers; binlog's reboot rotation carries both. Guard the localtime_r/gmtime_r NULL return, and make session_unique_basename fall back to a pid+nanosecond suffix rather than ever returning an occupied basename when the -2..-999 range is exhausted (which would truncate/append into another session's log). Found by review (codex).
Add a 'Name logs in a fixed timezone' checkbox (KEY_FLAG_USE_TZ) beside the offset field on the owner and admin edit forms, wired through both routes and templates. Also normalise an unsuffixed timestamp name to an implicit '-1' in the listing sort key so a same-second original lists before its '-N' collision siblings (lexically '-' < '.'). Found by review (codex).
Cover the timezone flag: settz enables use_tz and shows a fixed offset; clearing the flag reverts to server-local (date helpers stay local, the default). Add NaN-rejection, collision-suffix ordering, and webadmin use_tz enable/disable tests.
Address the second review pass (codex): - session_unique_basename now occupancy-checks every candidate, including the exhaustion fallback, so it can never return a name that would truncate/append another session's log. The fallback is a single numeric '-N' suffix (pid+nanoseconds concatenated), which the web UI's session regex already accepts — so fallback logs stay browsable. - session_time_strings clamps the offset to [-12,+14] before the time_t arithmetic, so a hand-edited record with use_tz set and an absurd offset can't overflow rather than just being non-finite-checked. - Correct the stale 'offset 0 = GMT/local' comments: the KEY_FLAG_USE_TZ flag is authoritative, not the value. Tests: occupancy-checked collision (harness-verified), pid+ns fallback browsable/downloadable, and an out-of-range offset clamped to +14h.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Renames session logs from
sessionN.{tlog,bin}to a start-timetimestamp
YYYY_MM_DD_HH:MM:SS.{tlog,bin}, and adds a per-entry,opt-in timezone that drives both the date subdir and the filename.
Naming
logs/<port2>/<YYYY-MM-DD>/<YYYY_MM_DD_HH:MM:SS>.{tlog,bin}..tlogand.binfor one session share a basename (computedonce at fork); the name is made unique up front (
-2,-3, … on asame-second collision) and every candidate is occupancy-checked so a
new session can never truncate or append into an existing log.
Timezone (opt-in per entry)
A new flag
KEY_FLAG_USE_TZgates a per-entry GMT offset(
tz_offset_hours, fractional hours):server's local time — unchanged from the pre-timestamp behaviour, so
existing installs need no migration.
0is genuine GMT).The record stays 168 bytes and fully forward/backward compatible — the
offset reuses a former reserved word and legacy records read back with
the flag clear.
Controls:
keydb.py settz <port2> <hours>sets the offset and enables theflag;
keydb.py clearflag <port2> use_tzreverts to server-localnaming without discarding the offset. Shown in
listastz=GMT+05:30 … use_tz.offset field, on the owner and admin edit forms.
legacy
sessionNnames, ordered chronologically.Notes
so deployments and their existing
sessionNlogs are unaffected.main.Testing
Full suite green (
scripts/run_tests.sh -j 16) plus the standalonebinlog/tlog/keydb/cleanup files. New coverage: timezone-in-the-name and
flag gating, keydb round-trip /
settz/ flag toggles, out-of-rangeoffset clamping, occupancy-checked collision handling and the fallback
name being browsable, and the web use_tz checkbox round-trip.
Reviewed twice with an independent tool; all findings addressed (fail-
closed collision names, non-finite/out-of-range offset handling,
gmtime/localtimeNULL guard, listing order).