Ldaprecord-laravel: [Bug] ldap:import --soft-delete crashes on large data sets

Created on 24 Sep 2020  路  3Comments  路  Source: DirectoryTree/LdapRecord-Laravel

Environment (please complete the following information):

  • LDAP Server Type: ActiveDirectory
  • PHP Version: 7.4

Describe the bug:

Binding 23,000+ variables to a prepared statement doesn't work well against a remote database.

https://github.com/DirectoryTree/LdapRecord-Laravel/blob/master/src/Import.php#L469...L479

        $guids = $this->imported->pluck($db->getLdapGuidColumn())->toArray();

        // Here we'll soft-delete all users whom have a 'guid' present
        // but are missing from our imported guid array and are from
        // our LDAP domain that has just been imported. This ensures
        // the deleted users are the ones from the same domain.
        $deleted = $db->newQuery()
            ->whereNotNull($db->getLdapGuidColumn())
            ->where($db->getLdapDomainColumn(), '=', $domain)
            ->whereNotIn($db->getLdapGuidColumn(), $guids)
            ->update([$db->getDeletedAtColumn() => $deletedAt = now()]);

In my scenario $guids resolves to an array of 23,000+ GUIDs. A prepared statement attempts to execute against a remote database but eventually fails after 5-10 minutes, likely because the database connection has timed out or the payload is too much.

Rough proposed solution which only hits the database in the scenario there are users to soft delete:
https://github.com/aanderse/LdapRecord-Laravel/commit/ce736b629fd9fec9edf252862a9a24683fc4c940

I hope I'm not missing any implications from switching from a whereNotIn to a whereIn with the update query, but please let me know if there is something I haven't considered.

bug

All 3 comments

@stevebauman any issues you can see with my proposed solution to this? Should I open a PR?

Hey @aanderse! Apologies for the super late response on this. I agree with your patch -- can you submit the PR and I'll put it through? Thanks a ton for your work on this 馃憤

Closing due to being resolved in #177. 馃憤

Was this page helpful?
0 / 5 - 0 ratings