Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11561 )
Change subject: WIP IMPALA-7626: Throttle partial RPC requests ...................................................................... Patch Set 2: (1 comment) It seems like this change limits the number of RPCs we can execute simultaneously for a specific endpoint. Would a simpler approach be to use a Semaphore to do the limiting? (A Guava RateLimiter class is also an option; they have related but different semantics.) You're always using "pool.submit(work).get()" so the calling thread is never doing anything while the RPC is outstanding, so I don't think you're getting a parallelization benefit from the pool. Regardless of which approach you take, please be sure to have timeouts on the pool stuff. If it gets stuck, we want the query to fail rather than hang indefinitely. I'll also note that the interesting throttle is on the catalog side. I don't think we're going to go there right now, but it's interesting to share the catalog-side queue of its RPCs fairly according to user or pool or whatever. Thanks! http://gerrit.cloudera.org:8080/#/c/11561/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/11561/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@277 PS2, Line 277: CatalogPartialRpcMgr.init(this, : BackendConfig.INSTANCE.getCatalogPartialRpcThreadpoolSize(), maxQueueLen); If you're going to initalize this here, why not just create it here, and not have a singleton? You can say this.partialRpcMgr = new ....() and avoid the singleton. -- To view, visit http://gerrit.cloudera.org:8080/11561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11f77a16cfa38ada42d8b7c859850198ea7dd142 Gerrit-Change-Number: 11561 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Tue, 02 Oct 2018 16:12:05 +0000 Gerrit-HasComments: Yes
