Abhishek Chennaka has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18866 )

Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:

http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/integration-tests/master_hms-itest.cc@886
PS4, Line 886:     ExternalMiniClusterITestBase::SetUp();
             :
             :     ExternalMiniClusterOptions opts;
             :     opts.hms_mode = GetHmsMode();
             :     opts.num_masters = 1;
             :     opts.num_tablet_servers = 1;
             :     opts.enable_kerberos = EnableKerberos();
             :     opts.principal = Principal();
             :     // Tune down the notification log poll period in order to 
speed up catalog convergence.
             :     
opts.extra_master_flags.emplace_back("--hive_metastore_notification_log_poll_period_seconds=1");
             :     opts.extra_master_flags.emplace_back("--logtostderr=false");
             :     StartClusterWithOpts(std::move(opts));
             :
             :     thrift::ClientOptions hms_opts;
             :     hms_opts.enable_kerberos = EnableKerberos();
             :     hms_opts.service_principal = "hive";
             :     ASSERT_OK(harness_.RestartHmsClient(cluster_, hms_opts));
> Alternatively, instead of copying over the implementation of several helper
Done


http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/integration-tests/master_hms-itest.cc@952
PS4, Line 952: With just 1 event created
> With just one event generated, ...
Done


http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/integration-tests/master_hms-itest.cc@953
PS4, Line 953: and hence creating more
             :   // than one event.
> nit: this part might be dropped?
Done


http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/integration-tests/master_hms-itest.cc@969
PS4, Line 969:   string line;
             :   bool log_found = false;
> nit: why not to move these into the ASSERT_EVENTUALLY scope below since the
Done


http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/master/hms_notification_log_listener.cc
File src/kudu/master/hms_notification_log_listener.cc:

http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/master/hms_notification_log_listener.cc@296
PS4, Line 296: Found the last seen event ID in the local Kudu master($0) "
             :                                   "to be greater than the latest 
event ID in HMS($1)
> How about:
Done


http://gerrit.cloudera.org:8080/#/c/18866/4/src/kudu/master/hms_notification_log_listener.cc@298
PS4, Line 298: Was there any backup or restore done on HMS recently?
> How about:
Done



--
To view, visit http://gerrit.cloudera.org:8080/18866
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 22 Sep 2022 01:28:52 +0000
Gerrit-HasComments: Yes

Reply via email to