Queued Geocodes infinitely loop causing database issues

Created on 29 July 2025, 5 days ago

Problem/Motivation

We first noticed this issue when our database reported it was full and we saw that the node_revision__field_location table took up 500MB.
After some investigation we discovered this was happening because when an address is geocoded in the queue it is incorrectly marked as needing resaving which then calls the presave hook and requeues that item. Further investigation lead me to FieldItemListInterface::equals in _geocoder_field_process. The problem there is that longitude and latitude values are stored in the database as strings while they are geocoded as floats and so in this instance they are reported as unequal (probably due to precision).

Steps to reproduce

Geocode an address in the United Kingdom (stored using the address module) into a geolocation field with queuing on in the settings and using google maps as the provider. Then run cron and see that the entity is processed and requeued in the queue table

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

4.29

Component

Code

Created by

🇬🇧United Kingdom aurora-norris

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @aurora-norris
  • 🇬🇧United Kingdom aurora-norris

    I can fix it by patching core with the fix for https://www.drupal.org/i/2925972 but I feel like there should be a less invasive way of fixing this.

  • 🇬🇧United Kingdom aurora-norris

    I've found better way to fix it, if just before you set the new field you get the current field and compare that both GeolocationItem will have data stored as floats and because they get their data from the same source they are exactly equal.
    A maintainer thinks this is a good way to solve this issue then I can make a merge request.

  • 🇮🇹Italy itamair

    Hi @aurora-norris thanks for reporting and inspecting all this, so deeply.
    Actually I am already inspecting and debugging this ... and indeed it looks that the GeocoderField @QueueWorker $entity->save() here:
    https://git.drupalcode.org/project/geocoder/-/blob/8.x-4.x/modules/geoco...
    is retriggering a new geocoder_field_entity_presave, at least once ... and then not a further time, as it looks the second time the $result === FALSE and then the $entity is not being resaved recursively ...

    Thus I was thinking we should mark/flag the saved $entity in some way as saved by the @QueueWorker and stop all the new geocoder_field_entity_presave in that case.
    I am going to inspect and implement this approach.
    What you think?

    Feel free to open your MR if you ended up to a better fix, so that I could review it, and compare, eventually.
    Feel also free to PM me (itamair) on Drupal slack for more immediate feedback.

  • 🇮🇹Italy itamair

    @aurora-norris it looks that I was quicker and opened a MR !70 with a solution that relies on drupal_static cache so to mark the entity as being saved by the geocoder queue and stop any further processing in the geocoder_field_entity_presave.

    That looks pretty logical and solid to me, and looks properly fixing this issue and stop further redundant entity save and revisions generations.

    Please QA and Review ... and let me know if some more work is needed on this (and why eventually).

  • 🇬🇧United Kingdom aurora-norris

    @itamair
    I've tested your merge request and it fixes my issue and processes the addresses that were being resaved every cron run.
    It looks like the better and more correct solution compared to mine.

    The code looks good and I've traced what happens on a normal entity save (where the entity is queued) and on a save by the QueueWorker and the new code does exactly what its meant to do.

    I'll make a MR with my solution in a minute so you can compare them because both our solutions issue at different levels.

  • Pipeline finished with Success
    4 days ago
    Total: 155s
    #560489
  • 🇬🇧United Kingdom aurora-norris

    Made a MR with my solution, feel free to not use.

  • Pipeline finished with Skipped
    4 days ago
    #560795
    • itamair committed 663e9606 on 8.x-4.x
      Issue #3538539: Queued Geocodes infinitely loop causing database issues
      
  • 🇮🇹Italy itamair

    Closing this as Fixed, with MR70! merged. Thanks @aurora-norris ... crediting you for this opportune catch.

  • 🇩🇰Denmark ressa Copenhagen

    This was a beautiful issue to find by chance, while checking changes in the latest release -- both the great detective work and thorough issue report with plenty of details by @aurora-norris, and @itamair coding and releasing a solution in less than a day. It's quite impressive, and I am very grateful to both of you.

Production build 0.71.5 2024