Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[HttpFoundation] Precalculate session expiry timestamp#33169

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@azjezz
Copy link
Contributor

QA
Branch?4.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?yes
Tests pass?yes
Fixed tickets#13955,#21423,#14625
LicenseMIT

Continued work from the original PR#21423 /#21857

@azjezzazjezzforce-pushed theprecalculate-session-expiry-timestamp branch from0aacdcd tocc3dc65CompareAugust 14, 2019 15:00
@azjezzazjezz marked this pull request as ready for reviewAugust 14, 2019 15:00
@azjezzazjezzforce-pushed theprecalculate-session-expiry-timestamp branch 12 times, most recently from3ac6cf7 to1d7737cCompareAugust 14, 2019 17:01
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedAug 15, 2019
edited
Loading

The migration path doesn't work IMHO.
Instead, I propose to keep the name of the column and of the property as is.
Then, on table creation, the column should be indexed, and we should use it to store absolute expiries. We should tell in UPGRADE files which query ppl need to run to add the index to their existing DB.
To have smooth compat with current sessions, to decide when a value in the column is a lifetime or an expiry, we would compare values to315576000 (10 years): when the number is higher, it's an absolute date, when it's ower, it's a relative date.
GC would be done in two DELETE queries, for >315576000 and <=315576000.
In v6, we would simply drop the <=315576000 one, because in two years we'll be able to consider that all relevant sessions have been migrated to absolute dates.

@nicolas-grekasnicolas-grekas added this to thenext milestoneAug 15, 2019
@azjezzazjezzforce-pushed theprecalculate-session-expiry-timestamp branch from1d7737c toe4f4a71CompareAugust 15, 2019 09:02
@azjezzazjezzforce-pushed theprecalculate-session-expiry-timestamp branch 7 times, most recently from37a924c to1e5586dCompareAugust 15, 2019 15:23
@azjezzazjezzforce-pushed theprecalculate-session-expiry-timestamp branch from1e5586d to9dd3b93CompareAugust 16, 2019 08:58
@azjezzazjezzforce-pushed theprecalculate-session-expiry-timestamp branch 4 times, most recently frome849cbb to0d57343CompareAugust 19, 2019 09:00
Co-authored-by: Benjamin Cremer <b.cremer@shopware.com>Co-authored-by: Rob Frawley 2nd <rmf@src.run>
@azjezzazjezzforce-pushed theprecalculate-session-expiry-timestamp branch from0d57343 to066000aCompareAugust 19, 2019 09:02
@nicolas-grekas
Copy link
Member

Thank you@azjezz.

nicolas-grekas added a commit that referenced this pull requestAug 20, 2019
… (azjezz)This PR was merged into the 4.4 branch.Discussion----------[HttpFoundation] Precalculate session expiry timestamp| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | yes| Tests pass?   | yes| Fixed tickets |#13955,#21423,#14625| License       | MITContinued work from the original PR#21423 /#21857Commits-------066000a [HttpFoundation] Precalculate session expiry timestamp
@nicolas-grekasnicolas-grekas merged commit066000a intosymfony:4.4Aug 20, 2019
@nicolas-grekasnicolas-grekas modified the milestones:next,4.4Oct 27, 2019
This was referencedNov 12, 2019
@tseho
Copy link
Contributor

tseho commentedNov 21, 2019
edited
Loading

Hello@nicolas-grekas, I think this is a BC break. On mysql, the lifetime column has been for the last few years a MEDIUMINT column.

The new version withtime() + $maxlifetime instead of$maxlifetime is higher than the MEDIUMINT mysql limit.

When upgrading from symfony 4.3 to 4.4 with an existing database, we have the errorSQLSTATE[22003]: Numeric value out of range: 1264 Out of range value for column 'sess_lifetime' at row 1

I saw the creation of the session table has been fixed 5 days ago in#34410 but that does not fix existing databases.

Is it normal we need to create manually a sql migration for a breaking database schema change, which is not really mentioned in UPGRADE-4.4.md ?

@nicolas-grekas
Copy link
Member

That's an interesting feedback@tseho but please create an issue: nobody is tracking comments on closed PRs.

@azjezzazjezz deleted the precalculate-session-expiry-timestamp branchNovember 21, 2019 13:30
@tseho
Copy link
Contributor

Thanks for your response and sorry about that, I just created a new issue :#34491

fabpot added a commit that referenced this pull requestSep 21, 2021
… session (IonBazan)This PR was squashed before being merged into the 6.0 branch.Discussion----------[HttpFoundation] [PDO] Don't fetch time when reading the session| Q             | A| ------------- | ---| Branch?       | 6.0| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       || License       | MIT| Doc PR        |Following up#33169 (comment)BTW is there a reason we are using `FETCH_NUM` instead of `FETCH_ASSOC` for more readability?Commits-------fb9508f [HttpFoundation] [PDO] Don't fetch time when reading the session
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+1 more reviewer

@TobionTobionTobion requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@azjezz@nicolas-grekas@tseho@Tobion@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp