Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/21128 )
Change subject: IMPALA-12264: Add limit on number of HS2 sessions per user. ...................................................................... Patch Set 2: (4 comments) Ideally we we might throttle any rogue application in TAcceptQueueServer::SetupConnection() which processes a new connection. The trouble is we don’t know the user until we run authentication, which is done by the TAcceptQueueServer::Task which processes thrift messages in a new thread. http://gerrit.cloudera.org:8080/#/c/21128/1/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/21128/1/be/src/service/impala-hs2-server.cc@491 PS1, Line 491: // Only check user limit if there is already a session for the user. > maybe we should pull this line at top of the function to avoid unnecessary I made this change. It means that we only keep track of the per-user count if max_hs2_sessions_per_user is set. This means that the new webui feature of being able to view "HiveServer2 Sessions by User" is not available unless max_hs2_sessions_per_user is set. I went back and forth and on this and decided this is maybe safest. http://gerrit.cloudera.org:8080/#/c/21128/1/be/src/service/impala-hs2-server.cc@492 PS1, Line 492: if (per_user_session_count_map_.count(user_name)) { > nit:we can replace it to 'load >= FLAGS_max_hs2_sessions_per_user' to avoid Thanks! http://gerrit.cloudera.org:8080/#/c/21128/1/be/src/service/impala-hs2-server.cc@493 PS1, Line 493: int64 load = per_user_session_count_map_[user_name]; > should we log some message here? and should we return the threshold to the Yes, good suggestion http://gerrit.cloudera.org:8080/#/c/21128/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21128/1/be/src/service/impala-server.cc@282 PS1, Line 282: "without receiving a status report before deciding that a backend is unresponsive " > nit: maybe -1 is better since it's used more to mean "unlimited", e.g. quer Done -- To view, visit http://gerrit.cloudera.org:8080/21128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd28edc352102d89774f6ece5376e7c79ae41aa8 Gerrit-Change-Number: 21128 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Xiang Yang <[email protected]> Gerrit-Comment-Date: Fri, 15 Mar 2024 23:57:32 +0000 Gerrit-HasComments: Yes
