Detect and configure a persistent session store by dmikusa · Pull Request #96 · cloudfoundry/php-buildpack

dmikusa · 2015-08-16T02:34:22Z

This PR adds code so the build pack will look at bound services and detect if a capable session store (currently redis or memcached) has been bound. If one is found, it configures PHP to use the service for session storage instead of the default which is the local file system.

PR Includes:

new build pack extension with session functionality
docs for usage
set of unit tests to confirm basic functionality

Additional testing can be done with a simple script such as this one.

<?php
    if (! session_start()) {
        print 'Session Start Failed :(<br/>\n';
    }
?>
<html>
<head>
    <title>Session Test</title>
</head>
<body>
    <h1>Current Session Id: <?php print session_id(); ?></h1>
    <h1>Current Node: <?php print $_SERVER["SERVER_ADDR"]; ?></h1>
    <h3>Session Dump</h3>
    <pre><?php print_r($_SESSION); ?></pre>
    <h3>Session Counter Test</h3>
    <p>This app increments a counter.  When multiple instances of the app are deployed, the counter should still increase properly.</p>
    <?php
        if (! array_key_exists('counter', $_SESSION)) {
            $_SESSION['counter'] = 0;
        } else {
            $_SESSION['counter'] += 1;
        }
    ?>
    <h3>Counter: <?php print $_SESSION['counter']; ?></h3>
</body>
</html>

When enabled the output should be a counter that increments and the Current Node should rotate.

cfdreddbot · 2015-08-16T02:34:24Z

Hey dmikusa-pivotal!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

cfdreddbot · 2015-08-16T02:34:24Z

Hey dmikusa-pivotal!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

cf-gitbot · 2015-08-16T02:34:26Z

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/101346342.

jtarchie · 2015-08-17T19:56:43Z

@dmikusa-pivotal is this feature dependent on this pull request?

dmikusa · 2015-08-17T19:57:39Z

Partly. The Redis support works without it, but to support Memcached it depends on that other PR.

davidjahn · 2016-03-28T20:46:09Z

Hey @dmikusa-pivotal , we are going through some old pull requests for the buildpacks.

Wanted to check in and try to understand better the purpose of this extension. Is it a common use case for developers to persist session information to redis or memcachd in PHP apps? If it's something you feel would be directly useful for a good number of php-buildpack users we'll make a story to merge in the PR (it looks like tests are failing right now because we've made some commits to develop since the PR was submitted).

dmikusa · 2016-03-29T12:42:26Z

Is it a common use case for developers to persist session information to redis or memcachd in PHP apps?

I'm not really sure. The use case would be for scaling up applications that use sessions, but using sticky sessions is not sufficient or appropriate. For example, if you have an application where the loss of the session could represent the loss of a substantial amount of work.

If it's something you feel would be directly useful for a good number of php-buildpack users we'll make a story to merge in the PR (it looks like tests are failing right now because we've made some commits to develop since the PR was submitted).

There were two main reasons I did this. A coworker asked me if it was possible and I had to tell him not at the moment and because the Java Build Pack does support it. It's probably a more advanced feature, but I do think that it would be genuinely useful to users.

Dan

…t store for PHP sessions

RochesterinNYC · 2016-04-07T15:49:41Z

we rebased the branch on top of develop then we realized we have to address cloudfoundry/binary-builder#8 before merging this pr.

shinji62 · 2016-04-08T00:23:55Z

I would like to say +1 for this PR.
12 fact application require share session storage so with that it become much more easy to setup session storage.

davidjahn · 2016-04-12T19:42:18Z

@dmikusa-pivotal & @shinji62 we partially merged the pr. The new release of the buildpack will have redis support. Regarding memcached support, we had a few questions regarding this pr which is a prerequisite for memcached support.

RochesterinNYC · 2016-04-25T15:28:40Z

We are packaging an appropriate version of libmemcached via this story and will be able to merge in the functionality of this PR once this is complete.

RochesterinNYC · 2016-04-26T19:01:23Z

We have investigated multiple avenues for including memcached sasl support and session storage via memcached into the php buildpack, and they both have drawbacks (listed below). At this point, and without explicit customer requests/use cases, we feel that the current memcached and Redis support is sufficient.

libmemcached: the PECL codebase appears to have minimal maintenance/upkeep; and compiling and pulling in libmemcached adds to our list of watched dependencies.
Native Support: Whilst memcachier/PHPMemcacheSASL works very well when calling it at the top of a PHP file; it appears to us to require too large an amount of work (to justify) to modify / wrap it to work as an SessionHandlerInterface, and thus be able to set it in the php.ini file.

We are closing this pull request. We are still open to discussion and would love to hear more about explicit use cases.

dmikusa · 2016-04-26T19:21:15Z

libmemcached: the PECL codebase appears to have minimal maintenance/upkeep; and compiling and pulling in libmemcached adds to our list of watched dependencies.

Can you expand on this? I'm not sure I follow the logic here. I'm wouldn't expect a lot of active development on libmemcached. My understanding is that it's more or less feature complete. Thus only updates are for bug / security fixes really.

As far as the PHP extension, it doesn't support PHP 7 now, but there does appear to be an active branch where it's being worked on: https://github.com/php-memcached-dev/php-memcached/tree/php7

As far as use cases, I think the main use case is pretty straightforward. If you want to share sessions across multiple instances of a PHP application you need somewhere to store the session data. The two common places are Redis & Memcached as they work very well for this type of thing. The PHP extension memcached has support for doing this. That said, you can store other things like cached data in memcached too. Having a solid extension for this would be nice too.

As far as the need to compile libmemcached from source, at the moment the Ubuntu version does not get built with support for libsasl which means you can only connect to memcached services that do not require authentication. Since most (all?) public providers will require authentication, you need to compile libmemcached & the PHP extension w/support for libsasl.

It seems like we're really close to having this working and into the build pack. It would be a shame to stop short now.

Dan

Daniel Mikusa added 2 commits April 7, 2016 11:32

Added extension to configure either redis or memcached as a persisten…

f4f5b1c

…t store for PHP sessions

Added documentation for new session feature.

86976a1

RochesterinNYC force-pushed the session-stores branch from b2190f4 to 86976a1 Compare April 7, 2016 15:48

RochesterinNYC closed this Apr 26, 2016

RochesterinNYC mentioned this pull request Apr 26, 2016

Build the PHP memcached extension with sasl support. cloudfoundry/binary-builder#8

Closed

RochesterinNYC deleted the session-stores branch August 3, 2016 15:44

Conversation

dmikusa commented Aug 16, 2015

Uh oh!

cfdreddbot commented Aug 16, 2015

Uh oh!

cfdreddbot commented Aug 16, 2015

Uh oh!

cf-gitbot commented Aug 16, 2015

Uh oh!

jtarchie commented Aug 17, 2015

Uh oh!

dmikusa commented Aug 17, 2015

Uh oh!

davidjahn commented Mar 28, 2016

Uh oh!

dmikusa commented Mar 29, 2016

Uh oh!

RochesterinNYC commented Apr 7, 2016

Uh oh!

shinji62 commented Apr 8, 2016

Uh oh!

davidjahn commented Apr 12, 2016

Uh oh!

RochesterinNYC commented Apr 25, 2016

Uh oh!

RochesterinNYC commented Apr 26, 2016

Uh oh!

dmikusa commented Apr 26, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants