Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token ...................................................................... Patch Set 20: (2 comments) http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc File src/kudu/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1073 PS14, Line 1073: } > I'm not sure I understand what you mean by injecting an assertio into the v Ah, sure. I meant: what happens if, say, an assertion for VerifyToken() triggers in one of the threads. Was curious how that'd behave in such a case. It's not a big deal, but was thinking whether the tests exists normally, reporting on the issue or it behaves some different way. http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc@960 PS20, Line 960: std::lock_guard<simple_spinlock> const l(jwt_by_account_id_map_lock_); Is it really necessary to hold this lock even after finding the element in the jwt_by_account_id_ dictionary? I'd expect that would be enough to guard just the access to the jwt_by_account_id_ dictionary, so those light-weight busy-wait locks would be very short in their lifetime. Especially given the fact that there are operations with EasyCurl below to make a remote call -- the latter might block for a really, really long time (especially in the context of busy-waiting synchronization primitives such as spinlocks). Ah, and another nit is that expressions like 'Type const x;' are valid, but don't follow the code style used in the project. -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 20 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Tue, 31 Jan 2023 01:55:08 +0000 Gerrit-HasComments: Yes
