search

8go / matrix-commander

simple but convenient CLI-based Matrix client app for sending and receiving
GNU General Public License v3.0
577 stars 60 forks source link

Feature Request: make it work well when several instances are run concurrently #31

Open expipiplus1 opened 3 years ago

expipiplus1 commented 3 years ago 
for i in $(seq 1 10); do matrix-commander --message $i & ; done

Running the above yields:

It would be excellent if multiple instances of this program could run concurrently, or at the very least wait politely for a while for any siblings to finish.

8go commented 3 years ago

Thank you @expipiplus1 for bringing this to my attention.

Yes, you are right. Concurrency is currently not supported and will cause errors and problems.

Making the matrix-commander concurrent would be quite some work.

Just as a workaround, for performance reason it is much better anyway to first prepare all messages and send all N messages with a single matrix-commander command.

Like: 

# some preprocessing, preparing all N messages
# calling matrix-commander similar to this:
matrix-commander --message $M1 --message $M2 ... --message $Mn
# this requires less resources and is way faster than calling matrix-commander N times
expipiplus1 commented 3 years ago

Sure, but sometimes that's not possible. For example when MC is being invoked concurrently from several scripts.

Is it not possible to do something simple like maintain a lock file on any contended resources?

On Thu, 20 May 2021, 11:02 pm 8go, @.***> wrote:

Thank you @expipiplus1 https://github.com/expipiplus1 for bringing this to my attention.

Yes, you are right. Concurrency is currently not supported and will cause errors and problems.

Making the matrix-commander concurrent would be quite some work.

Just as a workaround, for performance reason it is much better anyway to first prepare all messages and send all N messages with a single matrix-commander command.

Like:

some preprocessing, preparing all N messages

calling matrix-commander similar to this:

matrix-commander --message $M1 --message $M2 ... --message $Mn

this requires less resources and is way faster than calling matrix-commander N times

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/8go/matrix-commander/issues/31#issuecomment-845200019, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGRJXHDMHTOME3MMG3QTGDTOUP7TANCNFSM44EL2WFA .

8go commented 3 years ago

Sure, a "lockfile" or a PID-file is possible, maybe in combination with a flag/argument --wait which can be used as:

From the 2 variants lock-file vs PID-file the PID-file logic is already implemented.

So, overall what you want is possible. The missing part, logic, is the "check, sleep, recheck" logic.

It is not high on my priority list. Any nice soul out there that wants to contribute this feature as a PR?

8go commented 3 years ago

Reopened issue as Feature Request in hope to find a contributor for this feature.

expipiplus1 commented 3 years ago

Thanks for reopening. I wonder if there's some existing program which wraps other programs doing this lock file management. Probably wouldn't take too long to do in bash. Next time I bump into it I might take a look.

8go commented 3 years ago

Thanks for reopening. I wonder if there's some existing program which wraps other programs doing this lock file management. Probably wouldn't take too long to do in bash. Next time I bump into it I might take a look.

Yes, it could also be implemented OUTSIDE of matrix-commander as you point out correctly. You are right, a bash script wrapper could look for the presence of a PID file in the PID directory (e.g. /home/$USER/.run) and "pause" until no more PID files are in the directory.

Only a 99.9% solution because this would still allow 2 MC instances to start concurrently, both check at the same time, both find the dir empty of PID files, both start, .... and then possibly collide. A race condition.

expipiplus1 commented 3 years ago

well, that particular bug is easy to work around, after writing the pid file, only start if you read your own pid from that file. But overall you're right, concurrent programming is tricky :)

On Tue, May 25, 2021 at 10:31 PM 8go @.***> wrote:

Thanks for reopening. I wonder if there's some existing program which wraps other programs doing this lock file management. Probably wouldn't take too long to do in bash. Next time I bump into it I might take a look.

Yes, it could also be implemented OUTSIDE of matrix-commander as you point out correctly. You are right, a bash script wrapper could look for the presence of a PID file in the PID directory (e.g. /home/$USER/.run) and "pause" until no more PID files are in the directory.

Only a 99.9% solution because this would still allow 2 MC instances to start concurrently, both check at the same time, both find the dir empty of PID files, both start, .... and then possibly collide. A race condition.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/8go/matrix-commander/issues/31#issuecomment-847919803, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGRJXEMN7SLNG5QS73EONTTPOYDDANCNFSM44EL2WFA .

nirgal commented 3 years ago

@expipiplus1 As a workaround, you could wrap your calls with flock. I mean rather than run "matrix-commander args" run instead "flock -c mylockfile matrix-commander args"... This will delay the execution until the lock is available

expipiplus1 commented 3 years ago

yeah, that's a pretty good idea. thanks

On Tue, Aug 31, 2021 at 3:54 PM nirgal @.***> wrote:

@expipiplus1 https://github.com/expipiplus1 As a workaround, you could wrap your calls with flock. I mean rather than run "matrix-commander args" run instead "flock -c mylockfile matrix-commander args"... This will delay the execution until the lock is available

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/8go/matrix-commander/issues/31#issuecomment-909313336, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGRJXHC44IMGYRKGZ3XJMTT7TUJRANCNFSM44EL2WFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

xurizaemon commented 2 years ago

Another tactic would be to run parallel instances of matrix-commander in containers, eg via docker matrixcommander/matrix-commander. Containers could be given whatever shared state is required via a shared mount, and at the same time isolated from interfering with each others running.

8go commented 2 years ago

Thank you all @nirgal @expipiplus1 @xurizaemon for adding your excellent ideas. Please keep sharing. So, others can learn and take some clues, or even implement some of your suggestions.

For other people: Keep it going and please keep adding more suggestions if you have any! They are welcome! Thanks.

8go commented 2 years ago

See https://github.com/8go/matrix-commander-rs

An idea to improve performance :smile:

Maybe Rust also allows for more concurrency.

Talk about it to your friends, post about it, spread the word and provide PRs. :pray:

And give it a :star: if you like the idea.

edwinsage commented 1 year ago

If running concurrently is a goal (for example to speed up making numerous requests), you should also be able to achieve it by creating multiple credentials/stores and having each process use a different one, as described here: https://github.com/8go/matrix-commander/issues/86#issuecomment-1198493742

8go commented 1 year ago

If running concurrently is a goal (for example to speed up making numerous requests), you should also be able to achieve it by creating multiple credentials/stores and having each process use a different one, as described here: #86 (comment)

Correct. Should be working today.

Benjamin-Loison commented 1 year ago

I personally made sure to have a single matrix-commander installed with:

sudo find / -type f -name 'matrix-commander' 2> /dev/null

Then I renamed the given matrix-commander to matrix-commander_without_flock and I've added the following matrix-commander with +x permission:

#!/bin/bash

flock /var/lock/matrix-commander "$( cd "$( dirname "$0" )" && pwd )"/matrix-commander_without_flock "$@"

Thanks to @nirgal comment for using flock. That way I don't modify my scripts but only the matrix-commander program. After this change @expipiplus1 command line works fine (after adding parenthesis around matrix-commander --message $i & otherwise I get bash: syntax error near unexpected token `;').

Note that Docker still suffer this issue and I'm currently unable to make flock work as wanted with Docker. My approach was to change for something like this:

-ENTRYPOINT ["/bin/python3", "/app/matrix_commander/matrix-commander" ,"-s", "/data/store", "-c", "/data/credentials.json"]
+ENTRYPOINT ["/bin/flock", "/dev/shm/matrix-commander", "/bin/python3", "/app/matrix_commander/matrix-commander" ,"-s", "/data/store", "-c", "/data/credentials.json"]

in docker/Dockerfile.

However a host level flock, as done above, solves the concurrency issue with:

#!/bin/bash

flock /var/lock/matrix-commander docker run --rm -tv /my/chosen/folder:/data:z matrix-commander "$@"
8go commented 1 year ago

Thank you @Benjamin-Loison for sharing this with everyone.

jotwg commented 8 months ago

I'm using 'task spooler' (https://manpages.ubuntu.com/manpages/xenial/man1/tsp.1.html) to queue matrix-commander invocations. Works pretty good for sending notifications from my icinga2 monitoring system via matrix.