Conversation
I'm not loving this, but it works well enough to unblock.
NOTE: sys_user_limits is a *stub* table, the shape and contents of this table will almost definitely change in the very near future. Consider this as a placeholder until scope+limit-keys are wired up completely.
Expose vqueue entries as a single DataFusion table with stage-aware scanning. When the query filters `stage`, only matching stage key kinds are scanned; without a stage filter, all inbox stages are scanned and merged. Also project the latest entry metadata for observability (status plus EntryStatistics timestamps and counters), and add targeted tests for stage predicate extraction and sys_vqueues stage filtering behavior.
Baby steps towards unification and changing the ownership model of the invoker
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8bf69d53f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| timer_service, | ||
| scheduler_service, | ||
| invoker_handle, | ||
| invoker_task_guard.into_handle(), |
There was a problem hiding this comment.
Converting invoker_task_guard into a plain TaskHandle here removes drop-time cancellation, so a partition-processor panic/abnormal exit before step_down() will drop the handle without cancelling the invoker task. In this commit the manager’s abnormal-exit cleanup only unregisters introspection handles, so the leaked invoker can keep running detached until node shutdown, potentially processing stale in-flight work for a dead leader. Preserve a guard semantics (or add a drop-time cancel path) for this handle.
Useful? React with 👍 / 👎.
|
|
||
| use datafusion::arrow::datatypes::DataType; | ||
|
|
||
| define_sort_order!(sys_user_limits(partition_key, scope, l1, l2)); |
There was a problem hiding this comment.
sys_user_limits is declared as sorted by (partition_key, scope, l1, l2), but the rows come from UserLimiter::scan_counters iterating hashbrown::HashMap levels (scopes, l1, l2) with nondeterministic order and no sort before emission. DataFusion can rely on declared ordering to skip/add merge-sorting steps, so this can produce incorrect ORDER BY ... LIMIT behavior. Either sort rows before returning or remove the sort-order declaration.
Useful? React with 👍 / 👎.
1 participant
Stack created with Sapling. Best reviewed with ReviewStack.