Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13291 )
Change subject: [master_sentry-itest] one more scenario for authz cache ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/13291/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13291/3//COMMIT_MSG@11 PS3, Line 11: when Sentry is temporary shut down nit: to be more specific, 'when Sentry is temporary shut down and the master authz cache is enabled'. http://gerrit.cloudera.org:8080/#/c/13291/3/src/kudu/integration-tests/master_sentry-itest.cc File src/kudu/integration-tests/master_sentry-itest.cc: http://gerrit.cloudera.org:8080/#/c/13291/3/src/kudu/integration-tests/master_sentry-itest.cc@908 PS3, Line 908: / CreateTable() with operation timeout longer than HMS --> Sentry : // communication timeout successfully completes. After failing to push : // the information on the newly created table to Sentry due to the logic : // implemented in the SentrySyncHMSNotificationsPostEventListener plugin, : // HMS sends success response to Kudu master and Kudu successfully completes : // the rest of the steps. : ASSERT_OK(CreateKuduTable(kDatabaseName, kGhostTables[0])); : : // In this case, the timeout for the CreateTable RPC is set to be lower than : // the HMS --> Sentry communication timeout (see corresponding parameters : // of the MiniHms::EnableSentry() method). CreateTable() successfully passes : // the authz phase since the information on privileges is cached and no : // Sentry RPC calls are attempted. However, since Sentry is down, : // CreateTable() takes a long time on the HMS's side and the client's : // request times out, while the creation of the table continues in the : // background. Overall I feel scenario #1 is acceptable as the table is created successfully and Kudu returns Status::OK to the client, even though the owner privilege is not propagated to Sentry I think that is expected as Sentry service is down. Scenario #2 is something we might want to avoid, as the table is actually created and the client thought otherwise. Do you have some idea how we can prevent #2 from happening? Can we pull the sentry client time out config and validate if that is lower than Kudu's client timeout? http://gerrit.cloudera.org:8080/#/c/13291/3/src/kudu/integration-tests/master_sentry-itest.cc@927 PS3, Line 927: s.IsTimedOut( Hmm, I remember previously it failed with 'Already present' error, as the client timeout and retried. Any reason why it is failing with 'timeout' now. -- To view, visit http://gerrit.cloudera.org:8080/13291 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d3c04e4137afff407e4db8ee39a4495d9add3dc Gerrit-Change-Number: 13291 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 13 May 2019 17:40:00 +0000 Gerrit-HasComments: Yes
