Problem/Motivation

This has been split off from #2738051: \Drupal\views\Plugin\views\query\Sql::getCacheTags and getCacheMaxAge don't take into account that some entities can be NULL, which results in the same error, but the situation and proposed fix is different.

Entities that failed to load result in this error:

Error: Call to a member function getCacheTags() on null in Drupal\views\Plugin\views\query\Sql->getCacheTags() (line 1552 of core/modules/views/src/Plugin/views/query/Sql.php).

I did load testing for a site we are working on and we've actually seen two cases in our project how this can happen:

a) Persistent inconsistent data. In our case, this happened because an already deleted entity was re-saved again (#3029355: Deleting a node deletes the linked group_content entities which then resaves the node), then the query built by views does find the node but an entity load on it fails. I'll open a separate issue to prevent this specific case from happening, but there might be other issues that could lead to this.

b) A race condition. In my case, I was doing a load test where 20 users each created, updated and then deleted a new and finally went to the content overview page. I did that 5x in a loop and had two errors like this on the content overview page. So what I think happened is that the node was deleted *between* executing the view and loading the nodes.

Proposed resolution

At least for the second case, I think we should just drop records where we can't load the main entity completely. Even if we avoid errors on the Sql class, various field plugins for example fail with a similar error, e.g. BulkForm and EntityOperations. That could mean we are hiding problems in a), but we could combine it with a warning log message, if something happens once then it was just a race condition, if it happens on every request then you have to fix the data.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#26 3029434-nr-bot.txt90 bytesneeds-review-queue-bot
#24 Screenshot 2024-04-24 at 9.46.25 PM.png292.59 KBarunkumark
#23 3029434-nr-bot.txt90 bytesneeds-review-queue-bot
#16 3029434-16.patch15.34 KB_utsavsharma
#16 interdiff_14-16.txt1.11 KB_utsavsharma
#15 3029434-nr-bot.txt2.14 KBneeds-review-queue-bot
#14 views-missing-entities-3029434-14.patch15.39 KBberdir
#2 views-missing-entities-3029434-2-interdiff.txt5.48 KBberdir
#2 views-missing-entities-3029434-2.patch15.26 KBberdir

Issue fork drupal-3029434

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Berdir created an issue. See original summary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new views-missing-entities-3029434-2.patch15.26 KB
new views-missing-entities-3029434-2-interdiff.txt5.48 KB

Copied the patch from the other issue and also extended with a check and test coverage for relationships, as that currently reinitializes the results key as a stdClass and then that fails later...

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/views/tests/src/Unit/Plugin/query/SqlTest.php
@@ -312,6 +317,146 @@ public function testLoadEntitiesWithNoRelationshipAndNoRevision() {
+    $logger->warning('Failed to load entity @id for view @view. If this warning persists then it indicates that the database has inconsistent entity data.', [
+      '@id' => 1,
+      '@view' => 'test',
+    ]);

I do like the fact that you test this!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: views-missing-entities-3029434-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Reviewed & tested by the community

Testbot broken?

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/views/src/Plugin/views/query/Sql.php
@@ -1652,13 +1669,27 @@ protected function assignEntitiesToResult($ids, array $entities, array $results)
+            $this->logger->warning('Failed to load entity @id for view @view. If this warning persists then it indicates that the database has inconsistent entity data.', [

I think we should include the entity type in the error message, then you can go straight from log message without needing to know the view configuration.

rosk0’s picture

rosk0
he/him
Primary language Russian
Location Wellington
commented
Issue summary: View changes
Related issues: +#3029355: Deleting a node deletes the linked group_content entities which then resaves the node, +#3010896: Don't try to re-save deleted entities

Crosslinking, as those errors was just a symptom of the underlying data caused by linked issues.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new views-missing-entities-3029434-14.patch15.39 KB

@catch: That would be useful but would also make this a bit more complicated. We actually don't know the entity type in this method, we just get a list of entities and it's called twice from \Drupal\views\Plugin\views\query\Sql::loadEntities in separate blocks, so we'd need to add logging there in both cases if loading fails and then skip it without logging in this method. Worth it?

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3029434-nr-bot.txt2.14 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

_utsavsharma’s picture

_utsavsharma commented
StatusFileSize
new interdiff_14-16.txt1.11 KB
new 3029434-16.patch15.34 KB

Tried to fix CCF for #14.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Berdir opened merge request !6763

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review

Created a MR for 11.x

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

arunkumark made their first commit to this issue’s fork.

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Status: Needs work » Needs review

Updated the MR with references to the fix done on #2932518: Deprecate watchdog_exception

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3029434-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

arunkumark’s picture

arunkumark
Location Coimbatore
commented
StatusFileSize
new Screenshot 2024-04-24 at 9.46.25 PM.png292.59 KB

Verified, able to merge to 11.x verified in local.

Merge

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Status: Needs work » Needs review

To confirm bot execution keeping in NR.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3029434-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

arunkumark opened merge request !7719

arunkumark changed the visibility of the branch 3029434-loading-entities-fix to hidden.

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left comment on MR but also has test failures.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.