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

Authored by • max on May 23 2022, 5:27 AM.

Details

Reviewers

• karol
tomek

Commits

rCOMMeb8e77a14185: [services] Tunnelbroker - Introducing `deleteQueueIfEmpty` method for…

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

Branch

deleteQueueIfEmpty

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.May 23 2022, 5:27 AM

Herald added subscribers: • abosh, • karol, atul and 3 others. · View Herald TranscriptMay 23 2022, 5:27 AM

• max edited the summary of this revision. (Show Details)May 23 2022, 5:29 AM

• max added reviewers: • karol, tomek.

Harbormaster completed remote builds in B9381: Diff 12999.May 23 2022, 5:33 AM

• max requested review of this revision.May 23 2022, 5:33 AM

• max added a child revision: D3833: [services] Tunnelbroker - Put and get messages from database in grpc handler.May 23 2022, 6:20 AM

tomek accepted this revision.May 23 2022, 9:44 AM

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 marked an inline comment as done.May 25 2022, 7:08 AM

• 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.