Problem/Motivation

Fatal error when creating a new image field on a content type:

[28-Oct-2016 20:44:48 UTC] Uncaught PHP Exception PDOException: "SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'address' at row 1" at /srv/bindings/29e5ca7fd22d41b584facdaee72a22a6/code/vendor/lcache/lcache/src/DatabaseL2.php line 242

Proposed resolution

Not sure yet.

Remaining tasks

Investigate cause, write patch, write tests, etc.

User interface changes

none

API changes

none

Data model changes

none

Comments

Dustin LeBlanc created an issue. See original summary.

David Strauss’s picture

This is not actionable without a stack trace or at least the bin+cid being written. It's not even clear that it's an LCache bug; there are sanity limits to how long your bins and cids can be.

stevector’s picture

I misunderstood how core limits the length of cache ids for its own database backend. While cleaning up the module in preparation for DrupalCon David and I solved a few bugs in this pull request: https://github.com/lcache/drupal-8/pull/4/files

One of the changes there was to bump up the address column length from 255 to 512. Our working theory was that core had a way to sensibly trim cache ids down to 255 but because our address column is a combination of Drupal's bins and cache ids, the limit was being hit (a 255 CID + a bin of any length).

An additional bit of complexity: the bug we were chasing that prompted that change was a silent and inconsistent mismatch of address in the L1 and L2 cache. So we put in this strict requirement so that a future length bug would fail loudly. https://github.com/lcache/drupal-8/pull/4/files#diff-5f45581f0aab7347223...

So here we are with the louder fail caused by a too-long address. After looking around core some more I found that we had an incomplete understanding of how the trimming of CIDs to 255 happens. It happens in the DatabaseBackend Class: http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Cache/Databa... This module does not benefit from the logic in that class without extending it.

I see us as having at least three options:

  • 1. Rewrite our Backend class to extend the Drupal core database backend class. The main benefit here would be the code reuse of not having to duplicate the normalizeCid() method. I think this option is a bad idea because extending is coupling and one-fewer method on our class is not a big benefit.
  • 2. Copy normalizeCid() and it's usages into the LCache Backend class.
  • 3. Add a normlizeAddress() method to the LCache library.


David, I've heard you say often that we should be putting complexity into the library to keep the CMS implementations as simple as possible. We need hashing of extremely long CIDs somewhere. Should we put the hashing in the Drupal module or in the Library?

stevector’s picture

Title: Uncaught PHP Exception » Uncaught PHP Exception caused by long cache IDs
Priority: Normal » Major
David Strauss’s picture

1. Rewrite our Backend class to extend the Drupal core database backend class. The main benefit here would be the code reuse of not having to duplicate the normalizeCid() method. I think this option is a bad idea because extending is coupling and one-fewer method on our class is not a big benefit.

I think this would be an abuse of object-orientation, as the LCache implementation would override almost every single other method.

2. Copy normalizeCid() and it's usages into the LCache Backend class.

This seems viable, but I would alter the hash in use to a truncated SHA-512 for performance.

3. Add a normlizeAddress() method to the LCache library.

This would also be viable, but I prefer option two.

stevector’s picture

Status: Active » Needs review
stevector’s picture

Status: Needs review » Fixed

  • fcff1cc committed on 8.x-1.x
    Issue #2823361 by stevector, David Strauss: Uncaught PHP Exception...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.