Skip to content
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

Prevent concurrent transfers when multiples sources are available #36

Merged
merged 6 commits into from Aug 12, 2019

Conversation

@mraspaud
Copy link
Member

commented Aug 9, 2019

The current move_it_client doesn't discriminate when several sources are available for a file and request pushes for each of them. This leads to problems as the same file is written to from multiple sources.

@mraspaud mraspaud added the bug label Aug 9, 2019
@mraspaud mraspaud requested a review from pnuu Aug 9, 2019
@mraspaud mraspaud self-assigned this Aug 9, 2019
@mraspaud mraspaud marked this pull request as ready for review Aug 9, 2019
@mraspaud mraspaud requested a review from TAlonglong Aug 12, 2019

if already_received(msg):
timeout = float(kwargs["req_timeout"])
resend_if_local(msg, publisher)

This comment has been minimized.

Copy link
@mraspaud

mraspaud Aug 12, 2019

Author Member

@pnuu @TAlonglong would you be ok if I removed this line ? This basically resends the message if the file is in the file_cache (meaning it has already been processed) and that the files are pointing to the local machine. I don't think this is actually needed. What do you think ?

This comment has been minimized.

Copy link
@pnuu

pnuu Aug 12, 2019

Contributor

I'm ok with that. But note that I have only ran few tests so far, so don't put too much weight on my thougts 😁 Anyway, sounds like removing it won't be affecting our production in the future either.

This comment has been minimized.

Copy link
@TAlonglong

TAlonglong Aug 12, 2019

Collaborator

Sorry, I dont think I get the logic. Why would you resend the message if its already received and is local? Its a lots of thinks I dont get in trollmove, so maybe I just stay away :-)

This comment has been minimized.

Copy link
@mraspaud

mraspaud Aug 12, 2019

Author Member

No, you are right. I think it's some legacy stuff that I forgot to remove. I'll clean this up now.

Copy link
Collaborator

left a comment

Hm, in the get_msg_uid, why might the msg.data contain more than one file.

@mraspaud

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

@TAlonglong we use move_it_client to transfer SDR file collections too for example.

@TAlonglong

This comment has been minimized.

Copy link
Collaborator

commented Aug 12, 2019

@TAlonglong we use move_it_client to transfer SDR file collections too for example.

Ah, ok. Nice to know. I only have used it to transfere 1 file at the time.

Copy link
Collaborator

left a comment

This looks good to me

@mraspaud

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

@pnuu do you want to review this or should I just merge it ?

@pnuu

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

Go ahead and merge. Just got back from holidays, and in addition need to move to another office room, so won't have much time in the few next days. I'll complain later if something doesn't work 😀

@mraspaud mraspaud merged commit 6eba25e into pytroll:master Aug 12, 2019
1 check passed
1 check passed
stickler-ci No lint errors found
Details
@mraspaud mraspaud deleted the mraspaud:fix-concurrent-transfers branch Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.