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

Group messages together instead of letting them get dropped #60

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

finnboeger
Copy link
Contributor

If messages are attempted to be sent in too close proximity to each other instead group them and send them together.
I also changed the pencil to get its minimum delay from the server settings.
This introduces an issue if the delay is low and a user is e.g. attempting to fill out a shape and creating a long path to do so. The server will now happily pass along the changes to other connected clients but will not store it past a certain point (more precisely after MAX_CHILDREN).

How should we go about this? I see three possibilities:

  • We could obviously increase the default value for MAX_CHILDREN.
  • We could split the path and begin a new one when it exceeds MAX_CHILDREN, however this would circumvent the intention behind that setting.
  • We could stop the tool from drawing, giving the user some feedback.

I would go for the third option and add an entry to the wiki explaining that part of the configuration and recommending a higher MAX_CHILDREN value if a high MAX_TOOL_POLLING_FREQUENCY is set.

By opening a pull request, I certify that I hold the intellectual property of the code I am submitting, and I am granting the initial authors of WBO a perpetual, worldwide, non-exclusive, royalty-free, and irrevocable license to this code.

@finnboeger finnboeger force-pushed the feature/batch-messages branch from b8c59ac to c91fe63 Compare May 5, 2020 21:17
@lovasoa
Copy link
Owner

lovasoa commented May 9, 2020

I do agree that the current way we handle the load of messages is not ideal. However, I'm not sure grouping messages together over a single websocket message is the solution. There is certainly an overhead to decoding a message, but is it significant enough to justify that ? The initial goal of message dropping was to avoid overloading the server with too many messages, and to keep the size of boards manageable. Grouping messages together to send them as a single network message does not really help with either of these goals.

What we could do is:

  • handle message dropping at the board level instead of in individual tools, making it respect MAX_EMIT_COUNT and MAX_EMIT_COUNT_PERIOD
  • mark some messages, such as the first one and the last one in a shape as non-droppable,
  • simplify paths at creation time. This is more work to implement, but this would allow to both limit the server load and represent the drawn shape with higher fidelity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants