Over in #2248767: Use fast, local cache back-end (APCu, if available) for low-write caches (bootstrap, discovery, and config) we propose using CachedStorage for configuration so that is the web server has APCu installed we can take advantage of it. Config collections introduced the possibility of cache keys exceeding the 255 max. This has since been fixed to so can include the collection in the cache key.

CommentFileSizeAuthor
#4 interdiff.txt7.75 KBeffulgentsia
#4 2326203.4.patch15.33 KBeffulgentsia
#1 2326203.1.patch13.43 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Active » Needs review
FileSize
2326203.1.patch13.43 KB

We have pretty good test coverage of cached storage. Berdir++

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Almost wanted to RTBC, but spotted two issues:

  1. +++ b/core/lib/Drupal/Core/Config/CachedStorage.php
@@ -98,32 +85,40 @@ public function read($name) {
+      // Whilst the keys of $cache_keys should be the configuration names that
+      // would for implementation details on CacheBackendInterface::getMultiple()
+      // that we should not trust.

    This English sentence doesn't parse for me, and I don't understand what it tries to say. 

  2. +++ b/core/lib/Drupal/Core/Config/CachedStorage.php
@@ -278,4 +274,43 @@ public function getCollectionName() {
+  protected function createCacheKey($name) {
...
+  protected function createCacheKeys(array $names) {

    "create" is an actual operation, especially within the scope of a storage related service.

    Can we rename these to generate*(), please?

Status: Needs review » Needs work

The last submitted patch, 1: 2326203.1.patch, failed testing.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Status: Needs work » Needs review
FileSize
2326203.4.patch15.33 KB
interdiff.txt7.75 KB

Fixes test and addresses #2.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed 64bd363 on 8.0.x 
    Issue #2326203 by effulgentsia, alexpott: Fixed Config's cached storage...

Status: Fixed » Closed (fixed)

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