Scheduling jobs and waiting for other activities

[kovidgoyal]

Use a non modal dialog (i.e. do something like)

d = error_dialog(..., show=False)
d.setModal(False)
d.show()

[meme]

Works a treat!

Hmmm, now I'll have to find something else to tinker with this week until the updated code is released Good thing there's already a new suggestion for the plugin to add in.

[chaley]

The other thing you could do is wrap your done function in Dispatcher(). By default it is wrapped in FunctionDispatcher, which holds the job until the function returns. Dispatcher does not hold the job, so the job will finish. You should do this only if the done function does nothing that must be complete before another job runs.

[chaley]

@meme: here is the patch file. Kovid will probably integrate it later today, but it might be useful to try it now so I can know more about what is happening.

[meme]

@chaley

Thanks. That patch seems to fix the issue with the search box (for everyone else: the search limitation in the Device Tab was being cleared once I reread the books/collections back from the device after a Create Collections)

But it hasn't resolved the issue of checkmarks being unset/not set in the library/device view. I can clearly see the checkmarks getting applied/shown in the view and then they are removed once I call metadata_downloaded.


On a separate note - I've now wrapped my _completed function with Dispatcher so I no longer use Modal to change the dialog and it works fine.

[chaley]

@meme: please try changing line 791 in gui2.device.py to be
self.set_books_in_library(self.booklists(), reset=True)

EDIT: forget it. this is not the problem

[chaley]

@meme: don't bother with the change.

Are you calling metadata_downloaded directly? With what as the job result?

[meme]

I'm calling it as follows just after I kick off my create job:

        self.gui.device_manager.create_job(create_preview.create, self.Dispatcher(self._create_collections_completed), desc, args=[ self, preview ]),
        self.gui.device_manager.books(self.Dispatcher(self.gui.metadata_downloaded))
        self.gui.status_bar.show_message(desc, 5000)

[EDIT: hmmm, whats that "," doing there - now removed, and no difference]

[chaley]

The only way I can explain this is if the create collections job is corrupting the booklists, which seems unlikely. What happens if you comment out the create collections job but leave the device_manager.books job?

[chaley]

Another thing to try is to call

self.device_manager.get_device_information(FunctionDispatcher(self.info_read))

instead of device_manager.books. This is better in any event, because it will update the device free space values. It is also the sequence that calibre uses.

I am a bit concerned about using Dispatcher to call metadata_read. This permits more jobs to start and your plugin to go ahead and do whatever before the GUI booklists are updated. This creates race conditions in updating the booklists and ondevice. I think you should use FunctionDispatcher there.

[chaley]

An yet more thoughts:

Does self._create_collections_completed start any jobs? If so, then we can possibly have ordering problems between the job started by it and the job started by self.device_manager.books. The books job will probably run before any job started by create_collections_completed, which means that any updates that create_collections_completed makes to the device booklists will win.

You can test this by changing the Dispatcher back to FunctionDispatcher. If this fixes it, then instead of using a non-modal dialog box, fire off a queued signal to open your dialog. The signal will be processed after your method returns, ensuring that the dialog opens outside the job.

[meme]

Ok, let's see if I can do this in some kind of order:

The completed job only displays a dialog for the user - no new jobs.

1. Original code. Search text entered for 1 author to limit list. In device tab: book entry appears with checkmarks and no collections listed, then checkmarks disappear and collections appear but entry still shown.

        self.gui.device_manager.create_job(create_preview.create, self.Dispatcher(self._create_collections_completed), desc, args=[ self, preview ])
        self.gui.device_manager.books(self.Dispatcher(self.gui.metadata_downloaded))

3. Without create collections - same as original

        self.gui.device_manager.books(self.Dispatcher(self.gui.metadata_downloaded))

3. Changing original to use FunctionDispatcher - same as original

        self.gui.device_manager.create_job(create_preview.create, self.Dispatcher(self._create_collections_completed), desc, args=[ self, preview ])
        self.gui.device_manager.books(FunctionDispatcher(self.gui.metadata_downloaded))

4. Changing to the new suggested call - OK

        self.gui.device_manager.create_job(create_preview.create, self.Dispatcher(self._create_collections_completed), desc, args=[ self, preview ])
        self.gui.device_manager.get_device_information(FunctionDispatcher(self.gui.info_read))

So it looks like modifying the code to use more appropriate functions to update the info worked!

Thanks!

Oh, when I say the collections show up - its just simple hash based ones - the code to read all collections from the kindle isn't complete yet.

[chaley]

Originally Posted by meme

So it looks like modifying the code to use more appropriate functions to update the info worked!

My suspicion is that it works because of job sequencing. Using this function will run two jobs instead of one, with the second job added after the first. This gives whatever else is being run a chance to finish before the 'books' job starts.

@kovid: I think if we are going down the road of having plugins run private and 'standard' jobs, then we might need a way for the plugin to wait until all jobs are finished. The problem is that there is nothing that ensures that a serial-job chain is not interleaved with another serial-job chain. This could be done as a special "job" that fires only when the job queue is empty and all Dispatch-style 'done' functions have finished. Converting Dispatch to FunctionDispatch and running it from another thread would permit collecting the information.

[kovidgoyal]

Since all device jobs now use FD anyway, if a plugin wants to wait for the device jobs to complete al it has to do is poll gui.jobs_manager.has_device_jobs periodically.

[chaley]

Originally Posted by kovidgoyal

Since all device jobs now use FD anyway, if a plugin wants to wait for the device jobs to complete al it has to do is poll gui.jobs_manager.has_device_jobs periodically.

The issue is not 'wants to wait', it is 'it must wait'.

The problem is that even with FD, job sequences are not locked. It frequently happens that the done function of job 1 schedules job 2, and that the semantics *require* that job two run before anything else does. This is true for the sequences upload_books -> add_books_to_metadata -> upload_booklists and get_device_information -> info_read.

Consider meme's sequence. He kicks off the collections job and then immediately queues a 'books' job. If the collections job triggers a sequence, his books job will run in the middle of it because it is already queued. The inserted books job messes with the booklists, changes internal data structures and other things. When the next job of the sequence runs, it could erase all of this or get confused.

I suppose that I am really saying that a job sequence easily could need to be atomic. There are several like that in the device connection and book upload logic. Currently they are not atomic, and we cannot guarantee the results.

One way to fix this would be to put jobs queued by an FD at the head of the queue. I don't see quite how to accomplish that. Another would be to have job sequences set some kind of lock that the last job in the sequence removes, but this has problems with failure. Another way would be to eliminate job sequencing, instead having one job function sequence the bits manually using an FD. This might be the best approach for internal calibre device jobs.