diaspora/diaspora

Import shouldn't fail when MaxRetries is reached

Flaburgan opened this issue · 4 comments

tclaus commented

The interesting part are the last few lines in attached log.

.. repeat
/home/fla/diaspora/diaspora/app/workers/send_public.rb:11:in `block in perform'
/home/fla/diaspora/diaspora/app/workers/send_base.rb:13:in `schedule_retry'
/home/fla/diaspora/diaspora/app/workers/send_public.rb:10:in `perform'
... sidekick stuff...
/home/fla/diaspora/diaspora/lib/diaspora/federation/dispatcher/public.rb:19:in `deliver_to_remote'
/home/fla/diaspora/diaspora/lib/diaspora/federation/dispatcher.rb:54:in `deliver_to_subscribers'
/home/fla/diaspora/diaspora/lib/diaspora/federation/dispatcher.rb:29:in `dispatch'
/home/fla/diaspora/diaspora/app/models/account_migration.rb:119:in `dispatch'
/home/fla/diaspora/diaspora/app/models/account_migration.rb:34:in `perform!'
/home/fla/diaspora/diaspora/app/services/migration_service.rb:60:in `run_migration'
/home/fla/diaspora/diaspora/app/services/migration_service.rb:24:in `perform!'

Which means to me: sidekiq runs not in a separate process and stops after some tries. In a production environment a failing job will not stop the import.

@SuperTux88
Shouldn't in this case the exception be rescued anywhere?

Flaburgan commented

I'm sorry about the tests I did without Sidekiq running. I should restart doing them with Sidekiq now (and also last version of code including #8302 and #8298).

tclaus commented

Its OK - the more issues we have and can discuss, the more experience we all get.

SuperTux88 commented

It's normal that sidekiq jobs fail, and stuff like this is only a problem with single process mode during development. Single process mode is only meant to be used in development and never in production because it's known that some things fail if you use them (long running jobs that should run in the background and not inside a request, and federation stuff where you need multiple requests running which doesn't work if you only have one process/thread), but you usually don't use these problematic things during local development, and for all the rest single process mode is simpler and faster for local development.

Migration is one thing that needs sidekiq, if you try to import big real life profiles, so in case you want to test that, you can still enable sidekiq also locally. Small exports/imports to test a specific fix actually work also well without sidekiq.

So if this was only caused because it was tested without sidekiq, I think we can close this.