On Wed, Nov 5, 2014 at 6:49 PM, Mark Caudill <mark.caud...@grooveshark.com> wrote:
> > https://bugs.php.net/bug.php?id=68331 > > > > I was hoping the submitter (or one of their coworkers who commented on > > it) would reach out to the list themselves to get more information > > since I don't know the whole situation. I did ask them to... > > Sorry, I had an email written up to send out to the list but missent > it yesterday. Thank you for taking the time to write this up though! > > > Story thus far: > > Yasuo's session-lock-ini RFC [1] in February 2014 was (partially) > > approved - the relevant change being that it adds support for lazy > > writing. Optionally. There's a pull request with it that has NOT been > > merged; as far as I can tell it's because there was discussion after > > the vote raising problems and there was another RFC [2] (since > > withdrawn) to change the first one. I also vaguely remember there > > being merge problems between 5.6 and something? > > > > Unrelated and prior to all this was a commit [3] in August 2013 for > > request #17860 [4] that made session writes lazy for everyone. [see > > session.c] Not optionally. It went into 5.6. Apparently this caused > > problems for some people as they made 68331 a few days ago. I'm not > > entirely sure that the userland code couldn't be changed to deal with > > the lazy writing without requiring the crazy workaround they > > described, but whatever I don't know their codebase. > > So for us we'd like to be able to write the session during shutdown > and that (prior to PHP 5.6) means doing it in the write() handler > that we registered. In PHP 5.6 the only reasonable alternative in > userland is to do it in the close() handler instead. This is really > hacky and there are other quirks with doing it in close instead. > > We really have two issues with the lazy writing implementation: > - We can't really update the timestamp for the session cleanly > without changing where the write is happening. > - Data that we want to save independent of $_SESSION doesn't cause > the session to be marked as needing saved. That means we don't > get the callback to the write() handler. > > > > > I tried explaining that session-lock-ini is not yet merged, despite > > being approved and being listed under the PHP 5.6 heading at /rfc, and > > that the commit in question was entirely separate, to no avail. > > Meanwhile I did NAB the bug on the grounds that the change was > > intentional. > > I don't think anyone is arguing that it was unintentional. It > simply was a backwards incompatible change that happened before the > RFC and wasn't mentioned in the backwards incompatible list. > > The bug that the commit actually addressed was from 12 years ago and > wasn't even asking for the feature that got implemented to address > the bug. That made me suspicious of the route that commit took to > get into PHP 5.6. > > > > > So what's up with the RFC and its code? Is it still happening for 5.6 > > (too late?) or 5.7 (I lost track if we're doing that) or 7? Any > > suggestions for what I can tell the folks concerned? > > It seems that PHP 5.6 is under a feature lock so I'd be very > surprised if this was allowed in for PHP 5.6. Unfortunately, the best > route may be to revert the commit from 2013 since the change seemed > like it sidestepped the entire RFC process. > > > Here is my original email I was going to send: > It seems like the RFC for session lazy_write passed and was intended > for PHP 5.6 but didn't quite make it into the release. Is anyone aware > of the status of the RFC and what will actually happen as it was > approved but not implemented in time? [1] > > Similarly, there was a commit [2] related to doing a lazy_write on > session that appears to have been committed far before the RFC was > drafted but had unintended consequences. There is an unfortunate > backwards incompatible bug in this commit that custom session handlers > are not called when the session hasn't changed so anything outside of > the normal $_SESSION state isn't handled. In addition to that, this > has caused updating session timestamps to be more difficult and > require a slight rewrite of PHP implementations (ironically not > writing in the write() call). > > I have created a bug [3] for this but it got NABed due to the RFC but > it really appears to be stagnant at this point. PHP 5.6 is already > released and I feel like this has really manifested as a bug at this > point since it's actively causing issues with custom session handlers. > > [1]: https://wiki.php.net/rfc/session-read_only-lazy_write > [2]: > https://github.com/php/php-src/commit/554021d21e1b2517313a377676260c188152c2eb > [3]: https://bugs.php.net/bug.php?id=68331 > > Mark > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > Hi, thanks for the report. first let me clarify a couple of things: 1, neither of https://wiki.php.net/rfc/session-lock-ini or https://wiki.php.net/rfc/session-read_only-lazy_write made into 5.6 in their proposed forms 2, the changes about the write short circuit (and session_gc() and session_serializer_name()) and the lack of consensus went under the radar originally, as it was commited to master before 5.6. sometimes later the write short circuit one was spotted, and Yasuo was asked to make a proper rfc about it, which lead to the creation of session-lock-ini rfc which didn't got much attention originally, but the option which passed was basically the same thing which was committed before, plus an optional argument for session_start which controlled the behavior. this rfc was not followed up on though, and this was spotted by Andrey who pointed out the discrepancies about the session related changes and asked the list to reconsider the inclusion of those: http://grokbase.com/t/php/php-internals/143brgjp9d/revert-session-serializer-name-session-gc http://grokbase.com/t/php/php-internals/143fagz0a7/rfc-revert-extend-postpone-original-rfc-about-read-only-lazy-write-sessions http://grokbase.com/t/php/php-internals/143r244qtr/rfc-session-start-read-only-lazy-write-take-2 a lengthy discussion broke out and Yasuo reverted the changes about session_gc() and session_serializer_name() which was introduced without prior discussion, and there were some more discussion about the committed write short circuit and the accepted but not yet merged lazy_write, which also triggered the creation of https://wiki.php.net/rfc/session-read_only-lazy_write but in the end, nothing was done, neither of reverting the original short circuit nor properly exposing this to the userland session handler. We also had a last minute blocker issue because of the write short circuit( https://bugs.php.net/bug.php?id=67694) but I guess we were too focused on shipping 5.6 at that point to take a step back and reconsider reverting/refining this small performance improvement before the 5.6.0 final. I cc'ed Julien(the other RM for 5.6), Yasuo (the original author of the change in question) and Andrey(who was the most vocal and active person discussing the lazy write changes with Yasuo and the author of the https://wiki.php.net/rfc/session-read_only-lazy_write rfc), and I would like to hear your opinion on the situation at hand. Personally I think that we can have 3 options: 1. keep everything as is, including the perf gain for those using the default session handler and risk the potential problems for others (for example some cache/memcache based handlers could be expecting the session write handler to be called to not let active but unmodified sessions to be expired/purged from the cache). (we keep a potential BC break and if you need to work around you have to do some acrobatics) 2. introduce a better interface for custom handlers which allows them to write custom logic to decide if they want to write out the session or not. (this would still be a potential BC break for some people, but a better upgrade path, but it would also cause introducing new features in a minor version, which would cause for apps/libs to either depend on a minor version or support both versions in their custom session handler). 3. revert the write short circuit. (this would cause a small perf loss for most users - to the same level as it was before 5.6 -, but it would eliminate the BC break when migrating to 5.6, and it would allow us to take our time to have time to properly figure out the introduction of this feature including the interaction with the custom session handlers). I would vote for 3, -- Ferenc Kovács @Tyr43l - http://tyrael.hu
