Problem/Motivation

Tests in #2575105: Use cache collector for state show that this state key can easily become the largest state key, in two projects, it is 50% of the data in state.

It is also a a perfect use case for using its own collection. it is data that is only needed in rare cases in the backend, we store a list of data per project, so it is its own list and we have API's to store data per-project, which means it is a big overhead to load everything and save everything again for every project we update.

Proposed resolution

Migrate the data to a separate key_value collection and store it per project.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 locale-translation-status-2575105-2834580-10-interdiff.txt550 bytesBerdir
#10 locale-translation-status-2575105-2834580-10.patch16.48 KBBerdir
#6 locale-translation-status-2575105-2834580-6-interdiff.txt550 bytesBerdir
#6 locale-translation-status-2575105-2834580-6.patch6.16 KBBerdir
#4 locale-translation-status-2575105-2834580-4.patch6.16 KBBerdir
#2 locale-translation-status-2575105-2834580-2.patch2.83 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Active » Needs review
FileSize
locale-translation-status-2575105-2834580-2.patch2.83 KB

First patch, seeing some test fails, so something is quite right yet.

Status: Needs review » Needs work

The last submitted patch, 2: locale-translation-status-2575105-2834580-2.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
locale-translation-status-2575105-2834580-4.patch6.16 KB

Fixed the test and added an update function. I'm just deleting the old data, something we do as well in some cases, we can always refetch it.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 as a volunteer commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +D8MI, +sprint 
+++ b/core/modules/locale/locale.module
@@ -990,21 +986,14 @@ function locale_translation_status_delete_languages($langcodes) {
-  $status = locale_translation_get_status();
...
-  foreach ($status as $project => $languages) {
-    if (in_array($project, $projects)) {
-      unset($status[$project]);
-    }
-  }
-  \Drupal::state()->set('locale.translation_status', $status);
+  \Drupal::keyValue('locale.translation_status')->deleteMultiple($projects);

Nice cleanup!

I don't see any issues. I believe it's a great improvement. Adding to D8MI sprint (with tags)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Reviewed & tested by the community » Needs review
FileSize
locale-translation-status-2575105-2834580-6.patch6.16 KB
locale-translation-status-2575105-2834580-6-interdiff.txt550 bytes

Thanks, I unfortunately noticed one small issue, the update number should be 8301 I think.

Sutharsan’s picture

Sutharsan CreditAttribution: Sutharsan as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Looking at the existing hook_update_N in core, I conclude you are right with this numbering change.

Sutharsan’s picture

Sutharsan CreditAttribution: Sutharsan as a volunteer commented

Confirmed: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record
  1. We need a change record for this change.
  2. +++ b/core/modules/locale/locale.install
@@ -294,3 +294,22 @@ function locale_requirements($phase) {
+function locale_update_8301() {

    The correct number is 8300 - this is documented somewhere. 

  3. +++ b/core/modules/locale/locale.module
@@ -990,21 +986,14 @@ function locale_translation_status_delete_languages($langcodes) {
-  $status = locale_translation_get_status();
-
-  foreach ($status as $project => $languages) {
-    if (in_array($project, $projects)) {
-      unset($status[$project]);
-    }
-  }
-  \Drupal::state()->set('locale.translation_status', $status);
+  \Drupal::keyValue('locale.translation_status')->deleteMultiple($projects);

    Nice

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Needs review
Issue tags: -Needs change record
FileSize
locale-translation-status-2575105-2834580-10.patch16.48 KB
locale-translation-status-2575105-2834580-10-interdiff.txt550 bytes

Change record: https://www.drupal.org/node/2836959

Updated the version number.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

interdiff looks good.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 7147993 and pushed to 8.3.x. Thanks!

Removed a superfluous new line at the end locale.install on commit.

  • alexpott committed 7147993 on 8.3.x 
    Issue #2834580 by Berdir: Move locale.translation_status state key to a...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: -sprint

Yay, thanks @berdir.

Status: Fixed » Closed (fixed)

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