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

Improved handling of create API call #1050

Merged
merged 5 commits into from
Apr 22, 2024

Conversation

Ithanil
Copy link
Contributor

@Ithanil Ithanil commented Mar 4, 2024

During work on #1049, it turned out the handling of the create API call could use some improvements. These are especially useful in the context of the mentioned PR, but are independent of it.

Description

Changes:

  1. In bigbluebutton_api_controller.rb, begin with checking whether the meeting is already running and only when nothing is found, continue with find_available servers and meeting find_or_create_with_server. When an existing meeting is found, use the returned meeting information directly, instead of the same information that would have been returned from find_or_create_with_server.
    This is mostly an improvement in terms of logic (e.g. allowing for 2) ) and also a performance improvement for the create call (which frontends may repeat before every single join call). This may be slightly more relevant in the context of Support for tagged servers (ready for review) #1049, where the find_available routine becomes slightly more expensive.
  2. Only increment server_load if no meeting was found in 1), i.e. when a new meeting is actually created. To compensate, increment server_load on every join call. I think the previous implementation did rely on frontends always pairing join calls with a create call, to adjust the preliminary load values. In any case, the poller will take care of proper load values after a while.

Testing Steps

Existing automated tests + used in actual production deployment, in combination with #1049 .

Notes

  • A notable change in behavior is that before this patch, a create call would always fail if Server.find_available threw an error, even if the meeting would already exist in the database. With this patch it would indeed try to forward the create call to the corresponding server, even if the said server is not registered as available anymore. Before this patch it would do exactly the same in the end, but only if any other server was found in find_available, from which it then would fall back to the registered server for the meeting.

@ffdixon
Copy link
Member

ffdixon commented Mar 7, 2024

Thanks @Ithanil for the improvements. We're working to schedule time to review.

@ffdixon
Copy link
Member

ffdixon commented Mar 22, 2024

Thanks again @Ithanil, will have feedback for you soon.

@farhatahmad farhatahmad merged commit 6d52025 into blindsidenetworks:master Apr 22, 2024
1 check passed
@Ithanil Ithanil deleted the create-meeting_tweaks branch April 23, 2024 05:46
farhatahmad pushed a commit that referenced this pull request Jun 27, 2024
* in create call, now check whether meeting is already running before finding available servers

* even more streamlining of the create call handling

* now preliminarily increment server load in join call (too)

* add test for load increment on join call

* add RSpec version of test for load increment on join
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.

None yet

3 participants