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.

Sign up for GitHub

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

Jump to bottom

Delay ScheduleSend if OnMessage is in progress #88

Open
tigrannajaryan opened this issue May 26, 2022 · 4 comments · May be fixed by #144
Open

Delay ScheduleSend if OnMessage is in progress #88

tigrannajaryan opened this issue May 26, 2022 · 4 comments · May be fixed by #144
Assignees
@nemoshlag

Comments

@tigrannajaryan
Copy link
Member

OpAMPClient functions which update the state normally call ScheduleSend to deliver the changed state to the Server. When this happens while OnMessage callback is active we want to delay calling ScheduleSend and only call it once when OnMessage handler returns. This will avoid unnecessary messages going to the server.
@tigrannajaryan tigrannajaryan added the help wanted Extra attention is needed label Jul 14, 2022
@nemoshlag
Copy link
Member

I might be missing something, but wouldn't it be useful to add an OnMessageDone callback that will call ScheduleSend (and remove it from the OnMessage callback)?

@tigrannajaryan
Copy link
Member Author

I might be missing something, but wouldn't it be useful to add an OnMessageDone callback that will call ScheduleSend (and remove it from the OnMessage callback)?

That will make it the responsibility of the user. I would prefer that the Client implementation is responsible for this.

@tigrannajaryan
Copy link
Member Author

One way to implement this is to set a flag in SenderCommon while ProcessReceivedMessage() is being executed. When the flag is set ScheduleSend() should avoid triggering the sending and instead needs to remember that sending is pending (we need better names since it will "pending of pending").
When ProcessReceivedMessage() is about to exit it resets the flag in SenderCommon which triggers the sending process.

@nemoshlag
Copy link
Member

Thanks @tigrannajaryan, I'll be happy to take this one

@tigrannajaryan tigrannajaryan removed the help wanted Extra attention is needed label Oct 12, 2022
@nemoshlag nemoshlag mentioned this issue Oct 19, 2022
Closed
6 tasks
@nemoshlag nemoshlag linked a pull request Nov 7, 2022 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nemoshlag nemoshlag

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Disable scheduleSend when onMessage callback is running nemoshlag/opamp-go
2 participants
@tigrannajaryan @nemoshlag