SpartnerNL / Laravel-Excel

🚀 Supercharged Excel exports and imports in Laravel
https://laravel-excel.com
MIT License
12.24k stars 1.91k forks source link

[BUG] Cannot using multiple servers and import excel file using queue #2373

Closed mtawil closed 3 years ago

mtawil commented 5 years ago

Prerequisites

Versions

Description

I have multiple servers running the same application, and all runs queue worker pointing to one Redis cluster as a queue driver, sometimes when trying to import an excel file using queue way, I get this error File not found at path: laravel-excel-**********.

I also set the remote disk option as s3, and I am sure that all the configurations are correct.

Steps to Reproduce

It seems like the following jobs sometimes run on different servers:

I think that one of the first queues created an excel file locally then upload it remotely. In the next queue, specifically StoreQueuedExport had run on another worker and trying to get the same file locally again, and it fails because it can't find the file.

image

mtawil commented 5 years ago

Figured out the issue, it's because of AppendQueryToSheet job, it gets dispatched many times based on the chunk count, so when the queue worker is running on multiple servers, the job will be randomly running by another server and the initiated laravel-excel file will be not founded.

patrickbrouwers commented 5 years ago

Best to make sure only 1 worker can pick up those jobs?

mtawil commented 5 years ago

TL;DR:

The issue was coming from the RemoteTemporaryFile class that using a getLocalPath and called the same method on LocalTemporaryFile class: https://github.com/Maatwebsite/Laravel-Excel/blob/3.1/src/Files/RemoteTemporaryFile.php#L51

We should find a way or do something on this method to be entirely remotely.

TS;WM:

Let's reproduce it by installing a Laravel application on five servers, with php artisan queue:work running on all of it, and then we will set:

When we request to export, e.g. 10,000 records, the initiated job will be handled by the server 3, which is will be the first who create the excel file.

Let's go to the class who dispatch the AppendQueryToSheet: https://github.com/Maatwebsite/Laravel-Excel/blob/3.1/src/QueuedWriter.php#L164

As we can see, the AppendQueryToSheet job will be pushed 100 times in our case, and it will be distributed to multiple servers in random, including the server three which had initiated the file.

You can see that the other servers are asking for the local file, not the remote one, which is not existing.

That's it.

patrickbrouwers commented 4 years ago

Hm, how it should work is that the remote temporary file copies itself to a local file and after writing, copies itself back to the remote file. That's what the sync() method does.

moyvera commented 4 years ago

same issue here, for now do you have a workaround ?

I replicated this issue using laravel horizon with multi process and multi instances of my app.

mtawil commented 4 years ago

Hello @patrickbrouwers

Hm, how it should work is that the remote temporary file copies itself to a local file and after writing, copies itself back to the remote file. That's what the sync() method does.

That's not true; there is no sync when the queued job spreads in the other servers.

haihv96 commented 4 years ago

I have same issue. the problem is worker queue run on another server.

stale[bot] commented 3 years ago

This bug report has been automatically closed because it has not had recent activity. If this is still an active bug, please comment to reopen. Thank you for your contributions.