Page MenuHomePhabricator

[services] Tunnelbroker - Introducing `deleteQueueIfEmpty` method for DeliveryBroker
ClosedPublic

Authored by max on May 23 2022, 5:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 3, 3:18 PM
Unknown Object (File)
Fri, Oct 25, 12:06 AM
Unknown Object (File)
Fri, Oct 25, 12:06 AM
Unknown Object (File)
Fri, Oct 25, 12:06 AM
Unknown Object (File)
Oct 8 2024, 1:06 AM
Unknown Object (File)
Oct 8 2024, 1:06 AM
Unknown Object (File)
Oct 8 2024, 1:06 AM
Unknown Object (File)
Oct 8 2024, 1:06 AM

Details

Summary

Following the comment on D3833 introducing the deleteQueueIfEmpty method. The method purpose is if clientDeviceID messages queue is empty we don't
need to store folly::MPMCQueue for it and need to free memory to fix possible 'ghost' queues.

Linear task: ENG-1146

Test Plan

Run yarn run-tunnelbroker-service, service is built and run after.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek added inline comments.
services/tunnelbroker/src/DeliveryBroker/DeliveryBroker.cpp
69–76 ↗(On Diff #12999)

The name of this method explains clearly what happens inside, so I don't think this comment is beneficial here. I would even say that it's harmful, because it describes one specific use case of this method, but the method could be used in a lot of different contexts. I think that a lot better place for this comment is where this method is used - to explain why do we need to call it.

This revision is now accepted and ready to land.May 23 2022, 9:44 AM

The comment was removed to put it in the place where the method is used.

max added inline comments.
services/tunnelbroker/src/DeliveryBroker/DeliveryBroker.cpp
69–76 ↗(On Diff #12999)

The name of this method explains clearly what happens inside, so I don't think this comment is beneficial here. I would even say that it's harmful, because it describes one specific use case of this method, but the method could be used in a lot of different contexts. I think that a lot better place for this comment is where this method is used - to explain why do we need to call it.

Not sure about this, but I've removed the comment. Let's put it into the place where it will be used.

max marked an inline comment as done.

Rebase on master.