Repository navigation
Feature: Session windows - #994
Merged
Merged
Conversation
gwaramadze
force-pushed
the
feature/session-windows
branch
from
August 5, 2025 14:35
f4251a1 to
939518d
Compare
gwaramadze
commented
Aug 6, 2025
gwaramadze
force-pushed
the
feature/session-windows
branch
from
August 12, 2025 07:32
d68ff8c to
e30c97e
Compare
gwaramadze
marked this pull request as ready for review
September 17, 2025 08:11
|
Hi, this is a very promising PR! |
|
I am evaluating Quix for a project that requires this feature. Is there any update on it? |
Session windows fragmented long sessions, dropped valid out-of-order events, never merged sessions bridged by a late event, and scanned the whole partition on every expiry. Rewrite process_window around a two-neighbour probe and fix the surrounding state primitives. Semantics: - An event joins a session iff it is within one inactivity gap of either boundary; grace_ms no longer takes part in assignment, only in closing. An event is late iff ts < watermark - gap - grace, so the default grace_ms=0 still leaves a full gap of out-of-order tolerance. - An out-of-order event within one gap of two open sessions merges them. - Stored sessions are maximal, disjoint and never within one gap of each other, including a session starting at timestamp 0. - Session end is half-open (last event + 1), like every other window type. State primitives: - iter_windows / iter_prefixes for lazy, seek-based lookup, replacing the full-partition scans in both expiry paths. - Persisted expiry checkpoints so expire_by_key is O(log n + expired) and expire_by_partition is gated to O(1) per message in the common case. - iter_items(upper_bound=None) for unbounded forward iteration. Setting a lone lower bound makes the pinned rocksdict yield nothing, so bounds are now set both-or-neither; from_key carries forward correctness. - _validate_duration restored to end_ms <= start_ms. Aggregations: - BaseAggregator.merge() plus implementations on all nine aggregators and Reduce(merger=). Non-mergeable aggregations now raise InvalidOperation when a session window is defined rather than corrupting a merge. - Collectors need no merge: a merged session's range is the hull of both, so the timestamp-keyed range fetch already returns everything in order. Also renames timeout_ms to inactivity_gap_ms end to end and rewrites the session windows documentation, which described behaviour the code never had.
Brings the session windows work up to date with 62 commits of main, notably PR #1044 (early window expiration with triggers). Conflict resolutions: - state/types.py: took main's delete_window docstring. Both branches added the method independently; main's wording is the upstream style. - windows/time_based.py: kept main's _before_update/_after_update assignments. The __init__ parameters auto-merged, so dropping the assignments would have silently swallowed the new callbacks. - docs/windowing.md: kept the two session window bullets. Adapting to #1044's contracts: - FixedTimeWindow.__init__ now accepts and forwards before_update and after_update. This intermediary class is ours, so main's definitions passed the new kwargs straight into a signature that rejected them. - SessionWindow.process_window takes the new headers parameter. - SessionWindowDefinition rejects trigger callbacks with ValueError, mirroring main's own rejection for sliding windows. A bridging event matches up to two sessions, so before_update has no single "current value", and a trigger-forced early close would break the maximal, disjoint and non-adjacent guarantee sessions are built on.
A review of the session window implementation found nine correctness defects. Each has a test that was red against the previous commit. Prefix ambiguity in the state layer (two defects, one cause): window keys are `<prefix>|<start>|<end>`, so when one message key is a separator-extension of another (`user` and `user|123`) the extension's keys sort inside the shorter key's byte range. The unbounded upper bound therefore returned another key's windows tagged with the queried prefix - contaminating aggregates, emitting under the wrong key and deleting nothing - while the prefix enumerator seeked straight past the extended key, so its sessions were never closed, emitted or deleted. Bounded lookups were unaffected because an 8-byte integer bound always begins with zero bytes. Fixed by filtering on exact key length and enumerating prefixes linearly; no seek can skip one prefix's range without risking another's. The linear pass costs O(windows) rather than O(prefixes) and is guarded by a currently-failing test until the index CF lands. Sessions closed while still growable: lateness and closing shared one threshold, so a session was closed, emitted and deleted while events belonging to it were still being accepted, and those events then formed a second session adjacent to the first. Closing now waits a further inactivity gap - the latest an admissible event can arrive - so lateness stays at `watermark - gap - grace` and closing moves to `last event + 2*gap + grace`. Partition expiry: a null checkpoint means "sweep now", but the first partition message overwrote it with its own due time and skipped the sweep, deferring closes that were already due; the checkpoint is now only ever lowered. The partition watermark also ignored per-key history, re-accepting events inside already emitted sessions. Reaggregating a stored `None` crashed instead of initializing, and the per-key expiry cursor stranded sessions written below it when a larger grace period lowered the close threshold. `Reduce.mergeable` ignored an overridden `merge()`, so a subclass that implemented merging was rejected for not implementing it, and a conflicting override passed the definition-time check and would have failed mid-stream.
Rewrite the comments and test docstrings that referenced an internal spec and review rounds as plain statements of what each asserts; trim argumentative comments to the codebase's standard; drop a false claim that the partition expiry sentinel round-trips through the 8-byte timestamp encoding (it is stored through the value serializer). Mark test_expiry_scan_cost_grows_with_windows_per_key_not_with_key_count xfail(strict=True): the partition-mode sweep enumerates prefixes with a linear pass over stored window keys; an (end, prefix, start) index column family is the follow-up. Revert the unrelated SlidingCountWindowDefinition._get_name hunk, add the "New in v3.27.0" callout to the docs section, and correct the iter_prefixes docstring and the on_late docs snippet.
…n mode A window closed under `closing_strategy="partition"` was emitted keyed by the state-store prefix rather than by the message key. `as_state()` stores a non-bytes key as `serialize(key, dumps)`, so a `str` key `user-1` reaches the store as `b'"user-1"'`, and `expire_by_partition` yielded that prefix straight downstream. Switching closing strategy therefore changed the emitted key's value and type while the payload stayed correct, breaking every downstream group_by, join or keyed consumer silently. The key-scoped path was unaffected: it yields the key it was given. `WindowedPartitionTransaction.key_from_prefix()` reverses `as_state()`, and `expire_by_partition` uses it. Nothing the store writes changes, so existing state is read unchanged. The discriminator is the processed record's key type, which `as_state()` applies when writing. It is exact while a topic's key deserializer is type-stable - true of every built-in, and `group_by` re-keys through a JSON repartition topic. Persisting the key kind would remove the assumption, at the cost of an on-disk format change. Tumbling and hopping windows are affected; sliding windows reject partition mode and count windows have no closing strategy.
SessionWindow.expire_by_partition has its own sweep and passed the store prefix to expire_by_key as the emitted key, the same defect d8fd128 fixed for tumbling and hopping windows. It now maps the prefix back with key_from_prefix; the state lookup still uses the raw prefix.
Contributor
|
hi guys, sorry for late response, there were some staff changes. I am working on this PR to be merged. |
daniel-quix
approved these changes
Sep 23, 2026
Merged
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
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.
Session windows
Summary
Adds
StreamingDataFrame.session_window(): events of one message key are groupedinto sessions separated by more than
inactivity_gap_ms, with dynamic boundariesdriven by the data rather than a fixed schedule. Sessions support the full
aggregation surface (
.agg(),.sum(),.count(),.reduce(),.collect(),…), both closing strategies (
.final("key")/.final("partition")),.current()and the
on_latecallback.The first implementation on this branch fragmented long sessions, dropped valid out-of-order
events, never merged sessions bridged by a late event, and scanned the whole
partition on every expiry.
process_windowis rebuilt around a two-neighbourprobe, and the supporting state primitives are added alongside it.
Semantics
grace_msno longer takes part in assignment, only in closing. An event is lateiff
ts < watermark - gap - grace, so the defaultgrace_ms=0still leaves afull gap of out-of-order tolerance.
including a session starting at timestamp 0.
endis half-open (last event + 1), like every other window type.last event + 2 * gap + grace— thelatest an admissible event could still extend it.
before_update/after_update, PR Feature: Early Window Expiration with Triggers #1044) are rejected withValueError, mirroring the existing sliding-window rejection: a bridging eventmatches up to two sessions, so there is no single "current value", and a forced
early close would break the disjoint/non-adjacent guarantee.
State-layer changes
iter_windows(prefix, start_from_ms, start_to_ms=None, backwards=False)—lazy, inclusive-lower-bound, optionally unbounded above, merged with the
transaction's uncommitted updates. Replaces the materialising
get_windowsin the session hot path.
iter_prefixes()— streams the stored window keys once and yields each distinctmessage-key prefix, replacing the full
keys()scan in partition-mode expiry.get_expiry_checkpoint/set_expiry_checkpoint— persisted cursors, soexpire_by_keyisO(log n + expired)andexpire_by_partitionis gated toO(1)per message in the common case.RocksDBStorePartition.iter_items(upper_bound=None)— unbounded forwarditeration. The pinned
rocksdictyields nothing when only a lower bound is seton
ReadOptions, so the two bounds are now set both-or-neither andfrom_keycarries forward correctness;
backwards=Truewithout an upper bound raises.WindowedState.delete_window(start_ms, end_ms)— a session is re-keyed wheneverit is extended or merged, so the old key has to go.
BaseAggregator.merge()/mergeable, implemented on all nine built-inaggregators, plus
Reduce(merger=). A non-mergeable aggregation raisesInvalidOperationwhen the session window is defined.Regression tests
Every row fails on the code without its fix. All live in
tests/test_quixstreams/test_dataframe/test_windows/test_session.pyunlessanother path is given.
2 * gaptest_long_session_does_not_fragmentassert updated[0][1]["start"] == 1000— a new session starts mid-streamtest_partition_strategy_rejects_events_before_partition_watermarkon_latenever called; the event is folded into a live sessiongrace_ms=0dropped in-gap out-of-order eventstest_out_of_order_event_inside_gap_is_accepted_with_zero_graceon_late.call_count == 0fails; the event is reported latetest_bridging_event_merges_two_sessions,test_session_window_merge_sessions,test_bridging_event_merges_two_sessions_multiaggregationlen(stored_windows(...)) == 1fails — two sessions remaintest_session_starting_at_epoch_zero_extends[1000, 1001)is stored instead of extending[0, …)test_stored_sessions_are_never_within_the_gapendinclusivetest_session_end_is_exclusive,test_session_end_is_exclusive_collect_no_leaked_valuesend == 5001fails; the collect variant leaves one value undeletedgrace_mshad no type checktest_session_window_def_init_invalid_grace_typeTypeErrorraised forgrace_ms="invalid"test_non_mergeable_aggregation_rejected_at_definition_time,test_sessionwindow_reduce_without_merger_raisesInvalidOperationat definition time; the merge corrupts at runtimetest_windowed/test_transaction.py::TestIterWindows::test_prefix_leak_for_separator_extended_keytest_windowed/test_transaction.py::TestIterPrefixes::test_iter_prefixes_skips_separator_extended_keytest_close_before_shared_by_lateness_and_closing_allows_adjacent_sessionsNonecrashedtest_multiaggregation_reaggregation_of_none_value,test_merge_of_none_valueTypeErrorinside_aggregate_value/_merge_valueson the persistedNonetest_partition_checkpoint_none_defers_already_due_sessions{b"k1", b"k2"} <= expired_keysfails; the due sweep is deferredtest_partition_watermark_ignores_prior_key_mode_history[100, 106)is acceptedgrace_msgrewtest_cursor_not_invalidated_when_grace_regresses_across_reopen[95, 96)is never emitted, however far the watermark advancesReduce.mergeableignored an overriddenmerge()test_reduce_mergeable_true_when_merge_overridden_without_merger_kwarg(+…_builds)mergeable is Truefails; the definition raisesInvalidOperationReduceaccepted a conflictingmerger=+merge()overridetest_reduce_conflicting_merge_override_not_caught_at_definition_timetest_expiry_paths_do_not_scan_the_partitionget_windows/keysspies record callsiter_windowsexclusive lower bound / materialisingtest_windowed/test_transaction.py::TestIterWindows::test_inclusive_lower_bound,…::test_is_lazy_does_not_use_materializing_get_items_get_itemscalled for the first elementiter_itemscould not iterate unboundedtest_rocksdb_partition.py::test_iter_items_unbounded_upper_bound,…::test_iter_items_backwards_requires_upper_bounddelete_windowmissing fromWindowedStatetest_windowed/test_state.py::test_delete_windowAttributeErrorDocs
docs/windowing.md: new "Session Windows" section (!!! info New in v3.27.0)covering the assignment and closing rules, merging and its effect on
.current()consumers, the mergeable-aggregator list, the grace period, and both closing
strategies. Existing sections updated for the new window type: the
on_latelist, closing strategies, store naming (
session_window_30000) and thewindow-definition-update notes.
StreamingDataFrame.session_window,SessionWindowDefinition,SessionWindow,BaseAggregator.merge/mergeable,Reduce(merger=),WindowDefinition.reduce, and the newWindowedState/WindowedPartitionTransactionprotocol methods.docs/api-reference/*.mdisleft to the post-merge regeneration.
Partition-mode expiry emitted the store prefix as the message key
Found while running the feature on a real deployment, and fixed here because
session windows hit it the moment
closing_strategy="partition"is used.as_state(prefix=key)stores a non-byteskey asserialize(key, dumps), so astrkeyuser-1reaches the store asb'"user-1"'. Both partition-mode sweeps--
TimeWindow.expire_by_partitionfor tumbling and hopping windows, andSessionWindow.expire_by_partition-- then yielded that prefix downstream as themessage key, while the key-scoped path yielded the real key. Switching closing
strategy therefore changed the emitted key's value and type with a correct-looking
payload, breaking any downstream
group_by, join or keyed consumer silently.WindowedPartitionTransaction.key_from_prefix()reversesas_state()and bothsweeps use it. Nothing the store writes changes, so existing state is read
unchanged. The discriminator is the processed record's key type, which is what
as_state()applies when writing: exact while a topic's key deserializer istype-stable, which holds for every built-in, and
group_byre-keys through a JSONrepartition topic. Persisting the key kind would remove the assumption at the cost
of an on-disk format change.
Tumbling and hopping windows carry the same defect on
maintoday and are fixedby the same commit; sliding windows reject partition mode and count windows have
no closing strategy. Regression tests for all three window types are in
tests/test_quixstreams/test_dataframe/test_windows/test_partition_expiry_key.py,red before the fix.
Verified on a live deployment
A Quix Cloud pipeline ran a scripted event table through three deployments of one
application differing only by closing mode, with a real RocksDB state volume and a
real mid-run restart. Every emitted session was compared against expectations
derived by hand from the three rules above.
count 10, one session, no fragmentation[0, 1, 3, 2]in a single session, event-time orderedon_latefired once; no second session, no resurrectionpartition.current()supersession after a mergestart=1400000followed by one atstart=1200000for the same keycount 10, emitted exactly onceThe partition-mode key defect above was found by this rig, not by the unit tests:
those compare against the same prefix representation the code produces, so the
mismatch is invisible to them. After the fix the same rig, same data and same
oracle turned the partition arm from zero matched sessions to all matched, with
identical keys on the key-mode and partition-mode output topics.
Known limitation
A partition-mode expiry sweep enumerates prefixes with a linear pass over the
stored window keys, so its cost is
O(stored windows)rather thanO(prefixes). No seek can step past one prefix's key range without riskinganother's, because a SEPARATOR-extended message key's window keys sort inside the
shorter key's range. The checkpoint gate amortises the pass, and closed sessions
are deleted so only open sessions remain in the column family.
test_expiry_scan_cost_grows_with_windows_per_key_not_with_key_countis markedxfail(strict=True)and documents the bound an(end, prefix, start)indexcolumn family would restore. That index is the follow-up, not part of this PR.
Compatibility
No syntax newer than the repository's current Python floor is introduced; the
changed modules parse under it unchanged.