Uploaded image for project: 'Couchbase PHP client library'
  1. Couchbase PHP client library
  2. PCBC-139

PHP client needs fastlz compression to be compatible with legacy memcached clients

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.0.4
    • Fix Version/s: 1.1.0
    • Component/s: library
    • Security Level: Public
    • Labels:
    • Environment:
      With both php-ext-couchbase-1.0.5-centos55-x86_64.tar.gz and php-ext-couchbase-1.0.6b23_centos-5.5-x64.tar.gz. I will attach php -i output as a separate comment. Using php-pecl-memcached extension 1.0.0. libmemcached 1.0.4.

      Description

      The existing memcached clients for PHP (pecl-memcached and memcache) both default to using fastlz compression for values >= 2000 bytes.

      The couchbase client library doesn't have fastlz compression included. It doesn't provide a useful error message when it runs into this problem. Instead, it misinterprets the stored value, tries to allocate a huge block of memory and fails. The error it gives (look, e.g., in the PHP-FPM web-access.log) is:

      [01-Nov-2012 20:45:49 UTC] PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 8460104323416721361 bytes) in /usr/share/nginx/html/cb.php on line 32

      To reproduce, just set a value >= 2000 bytes with pecl-memcached client, and try to read it with Couchbase client. Here's a sample:

      <!DOCTYPE html>
      <html>
      <head>
      <meta charset="utf-8">
      <title>Couchbase Server PHP Test</title>
      </head>
      <body>

      <p>
      <?php

      $host = "10.4.2.15";
      $user = "Administrator";
      $password = "password";
      $bucketName = "default";
      $persistConnection = false;
      $cb = new Couchbase($host . ":8091",
      $user, $password, $bucketName, $persistConnection);

      $mc = new Memcached('Pool-1');
      if (count($mc->getServerList()) == 0)

      { // No servers, this pool needs to be configured $mc->addServer($host, 11211); //$mc->addServer("other host", 11211); }

      // Change this to 1999 and it will work
      $valLength = 2000;
      date_default_timezone_set('UTC');
      $val = date(DateTime::RFC2822) . ' ';
      $val .= str_repeat('X', $valLength - strlen($val));

      $mc->set("a", $val);
      echo("The value for 'a' is: ");
      var_dump($cb->get("a"));

      ?>
      </p>
      </body>
      </html>

      The correct behavior is for the Couchbase client to ship, out of the box, compatible with existing Memcached client implementations. It should include fastlz support directly, or else clearly document how to add such support without requiring any unusual compilations or other hoops. The fastlz code is very small and portable, and hasn't changed since 2007, so it should present a minimal maintenance issue.

      In addition, the PHP client documentation should clearly indicate what changes are needed in order for the client to be compatible with the other Couchbase clients (Java, .NET, etc.). Again, without requiring recompilation or extra hoops to jump through.

      No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

        Hide
        ingenthr Matt Ingenthron added a comment -

        Trond: can you take care of backporting this to 1.0.x please?

        Show
        ingenthr Matt Ingenthron added a comment - Trond: can you take care of backporting this to 1.0.x please?
        Hide
        TimSmith Tim Smith (Inactive) added a comment -

        More detail. I was personally unable to get the build of the latest PHP code to work with the beta2 release. I was hitting the error that is reported on http://www.couchbase.com/issues/browse/CCBC-126 :

        Warning: Couchbase::__construct() [couchbase.--construct]: failed to create IO instance in /usr/share/nginx/html/cb.php on line 18

        Building from sources let me get past that and actually test the build that the customer was trying. And following from that was a need to deal with LD_LIBRARY_PATH and symlink hacks.

        A backport of the fastlz compression fix to 1.0.x, with an official release of that, would be best for this particular user's needs. Can we get an ETA on when that will be available?

        Thanks,

        Tim

        Show
        TimSmith Tim Smith (Inactive) added a comment - More detail. I was personally unable to get the build of the latest PHP code to work with the beta2 release. I was hitting the error that is reported on http://www.couchbase.com/issues/browse/CCBC-126 : Warning: Couchbase::__construct() [couchbase.--construct] : failed to create IO instance in /usr/share/nginx/html/cb.php on line 18 Building from sources let me get past that and actually test the build that the customer was trying. And following from that was a need to deal with LD_LIBRARY_PATH and symlink hacks. A backport of the fastlz compression fix to 1.0.x, with an official release of that, would be best for this particular user's needs. Can we get an ETA on when that will be available? Thanks, Tim
        Hide
        trond Trond Norbye added a comment -
        Show
        trond Trond Norbye added a comment - For 1.0.x: http://review.couchbase.org/#/c/22590/
        Hide
        TimSmith Tim Smith (Inactive) added a comment -

        I've confirmed it to work (using the current 1.0.x branch from github.com/couchbase/php-ext-couchbase).

        I had to create a fake /usr/lib64/libvbucket.la to get it to compile: http://www.couchbase.com/issues/browse/CCBC-127

        I noticed that compress.c has this redundant (unused) definition:

        92 /* headers which claim an uncompressed size above this figure are bad */
        93 #define DECOMP_SANITY_LIMIT 0x40000000

        And this inaccurate comment:

        172 /**
        173 * sanity check, don't allocate over a GB, we should make this number
        174 * smaller though
        175 */

        Obviously minor cleanup stuff, nothing significant that I can spot.

        Would be great to get a 1.0.x package built and available for download with this (and other critical fixes, like PCBC-75) available to customers. Is there an ETA on when that can be done?

        Tim

        Show
        TimSmith Tim Smith (Inactive) added a comment - I've confirmed it to work (using the current 1.0.x branch from github.com/couchbase/php-ext-couchbase). I had to create a fake /usr/lib64/libvbucket.la to get it to compile: http://www.couchbase.com/issues/browse/CCBC-127 I noticed that compress.c has this redundant (unused) definition: 92 /* headers which claim an uncompressed size above this figure are bad */ 93 #define DECOMP_SANITY_LIMIT 0x40000000 And this inaccurate comment: 172 /** 173 * sanity check, don't allocate over a GB, we should make this number 174 * smaller though 175 */ Obviously minor cleanup stuff, nothing significant that I can spot. Would be great to get a 1.0.x package built and available for download with this (and other critical fixes, like PCBC-75 ) available to customers. Is there an ETA on when that can be done? Tim
        Hide
        ingenthr Matt Ingenthron added a comment -

        Packages are being uploaded to the site right now. Web page update to follow. Release notes will go up on Monday.

        Show
        ingenthr Matt Ingenthron added a comment - Packages are being uploaded to the site right now. Web page update to follow. Release notes will go up on Monday.

          People

          • Assignee:
            trond Trond Norbye
            Reporter:
            TimSmith Tim Smith (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 16h
              16h
              Remaining:
              Remaining Estimate - 16h
              16h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Gerrit Reviews

                There are no open Gerrit changes