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

Wait on existing polling request if it takes longer than 2 seconds. #2587

Merged
merged 1 commit into from
Apr 19, 2018

Conversation

ahmelsayed
Copy link
Contributor

Small update to ignore timer clicks if there is already an ongoing polling request for logs.

Before I was using a switchMap to cancel the old request if there is a new one due. The problem there is that a new request is always going to be due in 2 seconds, and if the worker is slow or overloaded, it can take more than 2 seconds to get the logs. In that case we will end up in a request/cancel loop and won't be able to catch up.

/cc @davidebbo

@ahmelsayed
Copy link
Contributor Author

I guess another issue that this has that the previous implementation didn't is if a request is stuck for whatever reason, we will never cancel it. that can be fixed with something like

.switchMap(() => 
        this._functionAppService.getLogs(this.context, this.functionInfo, null, true)
        // if request is done, or it's been 10 seconds
        .merge(Observable.timer(10 * 1000))
        .do(() => ongoingRequest = false))

but I'm not sure if that's worth it.

@davidebbo
Copy link

Sure, but if it gets stuck completely, chances are the container is not too healthy. If so, you probably won't be able to run your function, and the lack of logs is irrelevant :)

So I think it's better with your change, but we'll pound on it and see how it works.

Copy link
Contributor

@hartra344 hartra344 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

giphy 5

@ahmelsayed ahmelsayed merged commit 63a9332 into dev Apr 19, 2018
@ahmelsayed ahmelsayed deleted the ahmels-polling-fix branch April 20, 2018 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants