siddharthteotia commented on code in PR #10108:
URL: https://github.com/apache/pinot/pull/10108#discussion_r1067580351
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcMailboxService.java:
##########
@@ -44,16 +48,17 @@
* </ul>
*/
public class GrpcMailboxService implements MailboxService<TransferableBlock> {
+ private static final Duration MAILBOX_CACHE_EXPIRY = Duration.ofMinutes(5);
// channel manager
private final ChannelManager _channelManager;
private final String _hostname;
private final int _mailboxPort;
// maintaining a list of registered mailboxes.
- private final ConcurrentHashMap<String, ReceivingMailbox<TransferableBlock>>
_receivingMailboxMap =
- new ConcurrentHashMap<>();
- private final ConcurrentHashMap<String, SendingMailbox<TransferableBlock>>
_sendingMailboxMap =
- new ConcurrentHashMap<>();
+ private final Cache<String, ReceivingMailbox<TransferableBlock>>
_receivingMailboxCache =
+
CacheBuilder.newBuilder().expireAfterAccess(MAILBOX_CACHE_EXPIRY.toMinutes(),
TimeUnit.MINUTES).build();
+ private final Cache<String, SendingMailbox<TransferableBlock>>
_sendingMailboxCache =
+
CacheBuilder.newBuilder().expireAfterAccess(MAILBOX_CACHE_EXPIRY.toMinutes(),
TimeUnit.MINUTES).build();
Review Comment:
Yes I think we should probably consider making Operator extend AutoCloseable
or a related interface and the anchor point that creates the operator chain
should ensure closing all the resources associated with the operators.
For example, in future we may want to do fine grained memory management and
even decide how much memory to give an operator to execute under. So when the
operator chain is set up, this information is passed down to the operators in
the form of some OperatorContext etc. OperatorContext itself should be a
AutoCloseable.
This will also take care of the ownership aspect mentioned in the above
comment by @walterddr . So when the operator chain finishes execution and is
closed, as long as everyone implemented the AutoCloseable, they will free
resources either implicitly (if used in try resource) or explicitly in calls to
close().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
