-
-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple external model instances #743
base: master
Are you sure you want to change the base?
Multiple external model instances #743
Conversation
…:execute. Since opening the model can cause a significant delay if the external model is still busy, a debug message might help the user understand what's going on.
…llows a task to clean up temporary data if it is terminated prematurely when time runs out.
…native to the default ImageProcessingTask to parallelize the facerecognition task when multiple instances of the external model are available.
…MultipleExternalModelInstancesTask is used when the external model is selected as current model and the number of external model instances is greater than one (config setting by user). The SettingsService had to be added to the BackgroundService class in order to access the configuration.
…ww/html/custom_apps/facerecognition/lib/BackgroundJob/FaceRecognitionBackgroundTask.php#65"
…odelInstancesTask so they show up in Nextcloud's log.
…ask::execute stops if the first image(s) is/are skipped for some reason (e.g. because they are locked or already processed).
…that the external model URL must explicitly specify a port when the option external_model_instances_have_consecutive_ports is set to true. The regular expression to match the external model's port from its URL has been made a constant of the ImageProcessingWithMultipleExternalModelInstancesTask class and has been reworked to be more robust.
…ltipleExternalModelInstancesTask.
…ogrammatically. The system setting for the default port of the model instance has been removed.
…external model with the face:setup command. Additionally, add an option to explicitly print the current setup.
…inting the currently configured setup in SetupCommand.
Hi @36grad There are changes that really interest me, for example the changes in face:setup, but the rest of the implementation, although what you did is really great, adds a lot of unnecessary complexity (Why multiple ports? I never saw that in an api, but I understand what you wanted to do).
This is not true, although unfortunately I am not having time to implement it. 😞 We have to make just this change. You dare? 😃 |
OK to be honest I did not fully understand the last comment you have referred to, so I need to look into this a bit deeper... But from a first quick glance it sounds like it looks like it will also spawn multiple instances of the external model (but additionally act as a loadbalancer). Then there's still the question how you feed the images parallely to the external model? The have included the option to have the instance not only behind a load balance but accessible individually via conscutive ports because
Or did I misunderstand can one single background_job analyze multiple images in parallel when combining with this gunicorn thing for the external model you mentioned? Besides that: |
Hi, About the local background_job task that makes the calls to the external model unfortunately it is not parallel. The use of file-level locks allows us to run multiple instances of the same process, but without conflicts in processing. I just realized that I never posted this!. 🤦🏻♂️
This would be the way to use all the new options.. p.s: Although I would like to make it 100% php within the backgroudn_job command, there is no standard api for multiple processes in php. Ideally we should use pthreads (officially deprecated module in php) to make the calls. and therefore in the meantime this is a small hack to obtain the expected results- |
As you said, pthreads is deprecated so I had tried to use PHP parallel which is supposed to replace it I thinks. While I was able to build a docker image with PHP ZTS and install th eparallel module, the script crashed whenever I tried to start a simple thread - I must have still been doing something wrong. That is why I moved on to use asynchronous cURL requests in this pull request. This way, in a single PHP thread we can use multiple external model instances in parallel. In the end I guess your current implementation will achieve the same and it does not really matter if you schedule a bash script or the PHP task in cron. |
Summary: with this change multiple instances of the external model can be used to analyze multiple images in parallel.
Motivation:
Since the recent change that analysis now uses more than one core the external model is kind of pointless (because it's only analyzing one image in one thread / using one core).
That is especially sad if you have (for whatever reasons) a machine with a lot of cores and RAM that could easily analyze more than one image at one.
Enter "Multiple external model instances":
With this change you can use multiple external model instances in parallel. For that, the new
ImageProcessingWithMultipleExternalModelInstancesTask
has been added which brings together the code from the originalImageProcessingTask
and theExternalModel
and whips that up withcurl_multi_exec
to asynchronically send image analysis requests to the external model instances.Configuration:
All relevant settings on the facerecognition app side can be set using the
occ face:setup
command. Usage is displayed withocc face:setup -h
.The by far easiest solution is to use docker compose for the external model. There are only two changes to the
docker-compose.yml
required:The following configuration line will use port 8083 through 8090.
The following configuration line will result in a total of 8 instances of the external model.
Each replica will listen on subsequent ports, starting with the first port in above configured port range.
Changes to facerecognition:
I tried to make as little chanegs to the original code as possible.
Three things were unavoidable:
SettingsService
.FaceRecognitionBackgroundTask
to add a cleanup method in order to be able to react to timeout.BackgroundService
.Besides that, I modified the
SetupCommand
to have access to more settings and to show the current settings a bit nicer :-)Also I have added Nextcloud's internal logger to the
FaceRecognitionContext
so that we can send messages to the Nextcloud log file.Final words:
I have tried this on my server and on a machine with 16GB memory I can run 8 instances in parallel (with a temp image size of 1600x1200).