Skip to content

feat(core)!: worker mode support#2172

Open
brendt wants to merge 30 commits into
3.xfrom
implement-resettable
Open

feat(core)!: worker mode support#2172
brendt wants to merge 30 commits into
3.xfrom
implement-resettable

Conversation

@brendt

@brendt brendt commented Jun 17, 2026

Copy link
Copy Markdown
Member

Breaking changes

  • Change Tempest\Core\Kernel::shutdown() signature
  • Add Tempest\Database\Connection::inTransaction(): bool
  • Add Tempest\Database\Connection::ping(): bool
  • Add Tempest\Database\Connection::reconnect(): void

TODO

  • Rector for breaking changes

Comment thread packages/database/src/DatabaseInitializer.php
Comment thread packages/database/src/DatabaseInitializer.php Outdated
@brendt brendt marked this pull request as draft June 17, 2026 11:22
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

Benchmark Results

Comparison of implement-resettable against 3.x (452e201d6fc8a3994d3f987f0fbec3f6a743edd1).

Open to see the benchmark results
Benchmark Set Mem. Peak Time Variability
ViewRenderBench(benchExpressions) - 24.533mb 0.00% 408.570μs +5.63% ±1.96% +1.78%

Generated by phpbench against commit e11dedf

Comment thread packages/auth/src/Authentication/SessionAuthenticatorReset.php
@brendt

brendt commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

@laylatichy it took a little longer, but here's my draft PR to implement the resettable interface and provide persistent database connections. Would love if you could share your input and possibly tell me what I missed :)

@brendt

brendt commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

To be clear: the goal of this PR isn't to provide the worker application, I'll work on that in a separate PR. I first want to make sure I haven't missed anything obvious in the base implementation

@brendt

brendt commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

Also something that's missing compared to #1996 is the memoization reset. I don't think they need to be reset, since they only contain reflection data that never changes. In fact, I think it's better not to reset them, since resetting would decrease performance.

Comment thread packages/database/src/DatabaseReset.php Outdated
Comment thread packages/database/src/DatabaseInitializer.php
Comment thread packages/database/src/Connection/ConnectionReset.php Outdated
Comment thread packages/database/src/DatabaseInitializer.php
Comment thread packages/database/src/Connection/ConnectionInitializer.php Outdated
@laylatichy

laylatichy commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

worth noting DeferredTasks, right now you finish them in shutdown, resetting them is kinda tricky, might require different approach

and db connection reset is a bit tricky with transactions, on db connection from request crashes mid transaction ping() i think returns fine, next requests reuses that transaction, but might need validating, a lot of time passed since i digged into it

on the memo yeah it should stay no point in resting it with how it's used now

aside from that looks good

@brendt

brendt commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

and db connection reset is a bit tricky with transactions, on db connection from request crashes mid transaction ping() i think returns fine, next requests reuses that transaction, but might need validating, a lot of time passed since i digged into it

Wouldn't the right approach be that all transactions should be finished before ending the request? My first idea is to have a TransactionReset, which will simply throw an exception when there's still an open transaction. Is that too naive?

worth noting DeferredTasks, right now you finish them in shutdown, resetting them is kinda tricky, might require different approach

Good point. Should we support it though? I think an easier "solution" is to not allow deferred tasks in worker mode, and force users to use async commands instead. Deferred tasks are a lazy solution anyway for those who don't want to set up async commands, but I think it's fair to assume people who chose worker mode won't mind using the command bus. WDYT?

@brendt

brendt commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Edit: having said that, I realize we actually use deferred tasks to clean up sessions 😅 Ok I'll think about it some more

@brendt

brendt commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Oh, actually: defer will work as expected:

Defer uses fastcgi_finish_request() under the hood. FrankenPHP implements this function (even if we don't rely on FastCGI/FPM at all, we can, and do emulate this function).

source

That's good, it means I just have to refactor the shutdown function. 👍

@brendt brendt changed the title feat: implement resettable interface for worker mode feat!: worker mode support Jun 18, 2026
@brendt brendt changed the title feat!: worker mode support feat(core_!: worker mode support Jun 18, 2026
@brendt brendt changed the title feat(core_!: worker mode support feat(core)!: worker mode support Jun 18, 2026
@brendt

brendt commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Ok @laylatichy a couple more changes:

  • Applied all your review comments
  • Added a check for open transactions when resetting. I'm currently throwing an exception. Does that make sense? Would auto-commit be better? It feels weird to do that, especially since everything that's using Database instead of Connection or TransactionManager should auto-commit or close, so if you end up with an open transaction, something lower-level weird is going on and I'd say likely a bug
  • Added Tempest\Router\WorkerApplication that should be used in worker mode. This one never exits but instead resets the container after every run.
  • Deferred tasks are handled before reset, which FrankenPHP apparently has built-in support for, so I guess they should be fine?
  • Also added a couple more kernel events: SHUTTING_DOWN, RESETTING, and RESET; we don't use them, but I reckon they might be convenient for debugging. The full flow in worker mode is as follows: shutting_down > shutdown > resetting > reset.

Comment thread packages/router/src/Exceptions/HttpExceptionHandler.php Outdated
Comment thread packages/database/src/DatabaseInitializer.php
Comment thread packages/database/src/Connection/ConnectionInitializer.php
@xHeaven

xHeaven commented Jun 18, 2026

Copy link
Copy Markdown
Member
  • If a singleton definition is not an instance (it doesn't have to be) but a closure, ConnectionReset::reset() might crash the app.

@brendt brendt marked this pull request as ready for review June 19, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants