New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpFoundation] Added a migrating session handler #26096

Merged
merged 1 commit into from Apr 6, 2018

Conversation

Projects
None yet
7 participants
@rossmotley
Contributor

rossmotley commented Feb 8, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR symfony/symfony-docs#9496
  • gather feedback for my changes
  • submit changes to the documentation
  • update the changelog

When migrating to a new session handler on a live system, it's useful to be able to do it with no loss of session data. This migrating handler allows the sessions to be written to an additional handler to enable a migration workflow like:

  • Switch to migrating handler, with your new handler as the 'write only' one. The old handler behaves as usual and sessions get written to the new one.
  • After verifying the data in the new handler (and after the session gc period), switch the migrating handler to use your old handler as the 'write only' one instead, so the sessions will now be read from the new handler. This step allows easier rollbacks.
  • After verifying everything, switch from the migrating handler to the new handler
@Tobion

This comment has been minimized.

Member

Tobion commented Feb 11, 2018

This sounds interesting. But this will only migrate sessions that are accessed. How do you deal with session data from currently inactive users? I guess you still need to manually transfer the data from the old storage to the new one for those. Or you just let them expire after the migration phase.

@rossmotley

This comment has been minimized.

Contributor

rossmotley commented Feb 11, 2018

Hi @Tobion, we leave the migrating handler running for at least the session garbage collection period, as any sessions which have not been written to during that time are considered stale and may be removed by the session handlers themselves.

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Mar 22, 2018

@rossmotley is this PR ready on your side?

@rossmotley

This comment has been minimized.

Contributor

rossmotley commented Mar 26, 2018

@nicolas-grekas If there isn't anything to change with the code, then I think it's just adding something to the changelog and documentation. Should the changelog changes be added to this PR?

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Mar 26, 2018

The changelog of the component yes (not the root changelog as it is autogenerated)
If you could start a documentation PR, that'd be awesome.
ping @Tobion if you have some time for the review

@rossmotley rossmotley changed the title from [HttpFoundation] [WIP] Added a migrating session handler to [HttpFoundation] Added a migrating session handler Mar 26, 2018

@20uf

20uf approved these changes Mar 28, 2018

@Tobion

Some CS error but otherwise looks ok

/**
* @param \SessionHandlerInterface $currentHandler
* @param \SessionHandlerInterface $writeOnlyHandler

This comment has been minimized.

@Tobion

Tobion Mar 31, 2018

Member

this doc useless and should be removed

public function read($session_id)
{
// No reading from new handler until switch-over
$result = $this->currentHandler->read($session_id);

This comment has been minimized.

@Tobion

Tobion Mar 31, 2018

Member

no need for $result variable

private $currentHandler;
private $writeOnlyHandler;
public function setUp()

This comment has been minimized.

@Tobion

Tobion Mar 31, 2018

Member

this should be protected

public function setUp()
{
$this->currentHandler = $this->getMockBuilder(\SessionHandlerInterface::class)

This comment has been minimized.

@Tobion

Tobion Mar 31, 2018

Member

you can just use createMock here

/**
* {@inheritdoc}
*/
public function destroy($session_id)

This comment has been minimized.

@Tobion

Tobion Mar 31, 2018

Member

we use camelCase naming. please change the variable names

public function testWrites()
{
$sessionId = 'xyz';
$data = array('zfa' => 'hwq');

This comment has been minimized.

@Tobion

Tobion Mar 31, 2018

Member

the data passed to write is not meant to be an array, but a string

@rossmotley

This comment has been minimized.

Contributor

rossmotley commented Apr 5, 2018

@Tobion I have made the requested changes

@fabpot

fabpot approved these changes Apr 6, 2018

@fabpot

This comment has been minimized.

Member

fabpot commented Apr 6, 2018

Thank you @rossmotley.

@fabpot fabpot merged commit 3acd548 into symfony:master Apr 6, 2018

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Apr 6, 2018

feature #26096 [HttpFoundation] Added a migrating session handler (ro…
…ssmotley)

This PR was squashed before being merged into the 4.1-dev branch (closes #26096).

Discussion
----------

[HttpFoundation] Added a migrating session handler

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        | symfony/symfony-docs#9496

- [x] gather feedback for my changes
- [x] submit changes to the documentation
- [x] update the changelog

When migrating to a new session handler on a live system, it's useful to be able to do it with no loss of session data. This migrating handler allows the sessions to be written to an additional handler to enable a migration workflow like:

* Switch to migrating handler, with your new handler as the 'write only' one. The old handler behaves as usual and sessions get written to the new one.
* After verifying the data in the new handler (and after the session gc period), switch the migrating handler to use your old handler as the 'write only' one instead, so the sessions will now be read from the new handler. This step allows easier rollbacks.
* After verifying everything, switch from the migrating handler to the new handler

Commits
-------

3acd548 [HttpFoundation] Added a migrating session handler
@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Apr 6, 2018

We forgot implementing SessionUpdateTimestampHandlerInterface. Fixed in #26828.

Tobion added a commit that referenced this pull request Apr 7, 2018

minor #26828 [HttpFoundation] Have MigratingSessionHandler implement …
…SessionUpdateTimestampHandlerInterface (nicolas-grekas)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[HttpFoundation] Have MigratingSessionHandler implement SessionUpdateTimestampHandlerInterface

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Forgotten in #26096

Commits
-------

5d7117b [HttpFoundation] Have MigratingSessionHandler implement SessionUpdateTimestampHandlerInterface

@fabpot fabpot referenced this pull request May 7, 2018

Merged

Release v4.1.0-BETA1 #27181

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request May 24, 2018

minor #9496 Adding MigratingSessionHandler docs (rossmotley, javiereg…
…uiluz)

This PR was merged into the 4.1 branch.

Discussion
----------

Adding MigratingSessionHandler docs

Adding `MigratingSessionHandler` documentation for symfony/symfony#26096

Commits
-------

e93756f Added the versionadded directive
386f639 Reworded and simplified
7c8264d Update session_configuration.rst
e971074 Update session_configuration.rst
91aa58d Adding MigratingSessionHandler docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment