You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org> on 2022/08/19 04:06:18 UTC
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18866
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 67 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/18866/1
--
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: newchange
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18866 )
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
Patch Set 2:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/18866/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18866/2//COMMIT_MSG@15
PS2, Line 15: Was hesitant on crashing the
: master and chose to log an ERROR as in the case an out of order event
: id received from HMS might lead to unnecessary crash of the Kudu
: master
What parts of the system are still functional after this happening? I guess DDL operations do not work after this, but reading/writing data from existing tables is still possible, right?
Related question: what is the way to recover from this? Should there be a way to reset the latest HMS event identifier stored in the catalog in such case?
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/hms_catalog.h
File src/kudu/hms/hms_catalog.h:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/hms_catalog.h@147
PS2, Line 147: Status GetCurrentNotificationEventId(int64_t* event_id) WARN_UNUSED_RESULT;
nit: could this method be constant?
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.h@75
PS2, Line 75: // Delete the HMS database directory
nit: follow the style of the comments in the rest of this file -- add a period in the end.
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.cc@188
PS2, Line 188: RETURN_NOT_OK(Env::Default()->DeleteRecursively(JoinPathSegments(data_root_, metadb_subdir_)));
: return Status::OK();
nit: could be more concise
return Env::Default()->DeleteRecursively(...);
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@88
PS2, Line 88: opts.extra_master_flags.emplace_back("--logtostderr=false");
Maybe, create its own class to change this behavior just for the sake of a single test scenario? My concern is that with not writing into stderr it might be harder to troubleshoot test failures when tests are run via dist-test.
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@240
PS2, Line 240: rename it a couple of times to generate 3
Why is it needed? Why not to go with just 'default.a' -- that would be one HMS event already, right? Would be great to add comment to clarify.
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@263
PS2, Line 263: line.find("latest event ID in HMS(0)") != std::string::npos
Why not to search for the whole error message as at line 264?
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@271
PS2, Line 271:
nit: the indent seems to be messed up
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/catalog_manager.cc@1454
PS2, Line 1454:
nit: why is this change? I guess the 'if ...' line should have been shifted instead
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/catalog_manager.cc@1457
PS2, Line 1457: LOG(INFO) << "Last processed Hive Metastore notification event ID: " << hms_notification_log_event_id_;
Maybe, move this into
CatalogManager::InitLatestNotificationLogEventId() ?
Also, isn't this line too long?
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/hms_notification_log_listener.cc
File src/kudu/master/hms_notification_log_listener.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/hms_notification_log_listener.cc@295
PS2, Line 295: LOG(ERROR) << "Found the last seen event ID in the local Kudu master("<< processed_event_id <<
: ") to be greater than the latest event ID in HMS("<< event_id <<
: "). Was there any backup or restore done on HMS recently?";
nit: maybe, use Substitute() to make this more readable?
--
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: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 06 Sep 2022 23:13:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18866 )
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
Patch Set 7: Verified+1
unrelatest test failure: TsTabletManagerITest.TestTableStats (TSAN)
--
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: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 25 Sep 2022 12:32:24 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
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:
Looks like ASAN is still not happy. Any suggestions @Alexey?
"Bad status: Timed out: Timed out waiting for 1 TS(s) to register with all masters"
--
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 <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 14 Sep 2022 02:54:48 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
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 <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 22 Sep 2022 01:28:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
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: deleteVote
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18866
to look at the new patch set (#5).
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 80 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/18866/5
--
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: newpatchset
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin 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 methods from MasterHmsTest, I'd think of introducing a virtual method named 'PrepareCluster()' or something like that into MasterHmsTest to put there all this stuff, and call that method in the SetUp() method, while deriving MasterHmsDBTest from MasterHmsTest. Now, the newly added MasterHmsDBTest needs to override just PrepareCluster(), and there is no need to copy over the implementation of helper methods.
A better approach would be introducing a virtual method into MasterHmsTest to fill in the instance of ExternalMiniClusterOptions used in SetUp(), or just a method that produces the list of master flags. The the derived MasterHmsDBTest needs to override only that method.
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, ...
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?
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 they are not used anywhere else?
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:
The event ID $0 last seen by Kudu master is greater than $1 currently reported by HMS.
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:
Has the HMS database been reset (backup&restore, etc.)?
--
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 <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 14 Sep 2022 22:37:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18866
to look at the new patch set (#2).
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 68 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/18866/2
--
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: newpatchset
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
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 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc@892
PS5, Line 892: TestHMSDBErase
> It seems this test fails with ASAN warning. Since it's not clear what's go
Done
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc@914
PS5, Line 914: "than 0 currently reported by HMS. Has the HMS database "
: "been reset (backup&restore, etc.)?";
: ASSERT_EVENTUALLY([&] {
> nit: since this is a constant string, maybe move this to the outer scope, a
Done
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc@919
PS5, Line 919: std::ifstream in(strings::Substitut
> Isn't this always true since it's under the 'if (line.find(str) != std::str
Right, I guess that is a remnant of when we had part of the string in the if condition. Will get rid of it.
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/master/hms_notification_log_listener.cc
File src/kudu/master/hms_notification_log_listener.cc:
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/master/hms_notification_log_listener.cc@290
PS5, Line 290:
> Could you add a comment explaining why we are checking for that preconditio
Done
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/master/hms_notification_log_listener.cc@291
PS5, Line 291: // TODO(KUDU-2475): Ignoring errors could result in a client receiving an
: // ack for a table rename or drop which fails.
: WARN_NOT_OK(s, Substitute("Failed to handle Hive Metastore notification: $0",
: EventDebugString(event)));
:
: // Short-circuit when leadership is lost to prevent applying notification
: // events out of order.
: if (l.has_term_changed()) {
: return Status::ServiceUnavailable(
: "lost leadership while handling Hive Metastore notification log events", s.message());
:
> Since the code under 'for()' clause doesn't do anything to 'events', maybe
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: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 22 Sep 2022 23:28:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18866
to look at the new patch set (#6).
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 85 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/18866/6
--
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: newpatchset
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18866
to look at the new patch set (#4).
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 139 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/18866/4
--
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: newpatchset
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
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 3:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/18866/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18866/2//COMMIT_MSG@15
PS2, Line 15: Was hesitant on crashing the
: master and chose to log an ERROR as in the case an out of order event
: id received from HMS might lead to unnecessary crash of the Kudu
: master
> What parts of the system are still functional after this happening? I gues
Yes DDL operations especially drop/alter tables don't work. Create table works.
I think we can have a followup change list to introduce a kudu hms unsafe eventid <new event id. Default: 0> to recover from this situation.
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/hms_catalog.h
File src/kudu/hms/hms_catalog.h:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/hms_catalog.h@147
PS2, Line 147: Status GetCurrentNotificationEventId(int64_t* event_id) WARN_UNUSED_RESULT;
> nit: could this method be constant?
No, because the subsequent calls made in the body of the method cannot be const.
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.h@75
PS2, Line 75: // Delete the HMS database directory.
> nit: follow the style of the comments in the rest of this file -- add a per
Done
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/hms/mini_hms.cc@188
PS2, Line 188: return Env::Default()->DeleteRecursively(JoinPathSegments(data_root_, metadb_subdir_));
: }
> nit: could be more concise
Done
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@88
PS2, Line 88: StartClusterWithOpts(std::move(opts));
> Maybe, create its own class to change this behavior just for the sake of a
Done
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@240
PS2, Line 240: se("db"));
> Why is it needed? Why not to go with just 'default.a' -- that would be one
I found that if there is only one HMS event the event ID published by HMS is not 0. Not sure of the behavior of the HMS when there is only 1 event present. Hence created more than 1 event. Will comment that here.
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@263
PS2, Line 263: TR_CONTAINS(s.ToString(), kInvalidHiveTableError);
> Why not to search for the whole error message as at line 264?
Done
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/integration-tests/master_hms-itest.cc@271
PS2, Line 271:
> nit: the indent seems to be messed up
Done
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/catalog_manager.cc@1454
PS2, Line 1454:
> nit: why is this change? I guess the 'if ...' line should have been shifte
The return is in the 'if' block, right?
I'll shift the 'if' line and the corresponding ones as well.
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/catalog_manager.cc@1457
PS2, Line 1457: }
> Maybe, move this into
Done
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/hms_notification_log_listener.cc
File src/kudu/master/hms_notification_log_listener.cc:
http://gerrit.cloudera.org:8080/#/c/18866/2/src/kudu/master/hms_notification_log_listener.cc@295
PS2, Line 295: LOG(ERROR) << Substitute( "Found the last seen event ID in the local Kudu master($0) "
: "to be greater than the latest event ID in HMS($1)"
: ". Was there any backup or restore done on HMS
> nit: maybe, use Substitute() to make this more readable?
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: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 13 Sep 2022 17:51:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18866
to look at the new patch set (#3).
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 139 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/18866/3
--
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: newpatchset
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18866
to look at the new patch set (#7).
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 82 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/18866/7
--
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: newpatchset
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18866 )
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18866/6/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/18866/6/src/kudu/integration-tests/master_hms-itest.cc@893
PS6, Line 893: // TODO(achennaka): Skip test until server timeouts seen in ASAN builds are resolved.
: #if defined(ADDRESS_SANITIZER)
: GTEST_SKIP();
: #endif
It would be great to make sure ASAN tests pass: for some reason, dist-test could not submit the verification job: http://jenkins.kudu.apache.org/job/kudu-gerrit/26499/BUILD_TYPE=ASAN/console
It might be possible that ASAN still report an issue since the SetUp() runs before GSKIP_TEST(). Maybe, a simpler solution would be just leaving the test as DISABLED for a while, and then re-enabling when the issue with ASAN failure is resolved.
--
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: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 23 Sep 2022 12:29:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18866 )
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
[master] KUDU-3322 / KUDU-3319 HMS event logging
It would be useful to have additional logging in cases where HMS
notification events have to be analyzed from Kudu side. The Jiras
have more context on the reasoning behind this but this patch adds
* an INFO level log which shows the last processed HMS event id during
startup and
* an ERROR log if the latest event id from HMS is less than
the last seen event id from the master. Was hesitant on crashing the
master and chose to log an ERROR as in the case an out of order event
id received from HMS might lead to unnecessary crash of the Kudu
master.
Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Reviewed-on: http://gerrit.cloudera.org:8080/18866
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/hms_notification_log_listener.cc
7 files changed, 82 insertions(+), 3 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
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: merged
Gerrit-Change-Id: I242c9cedf170ed621867b905f1cb1875347d5887
Gerrit-Change-Number: 18866
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18866 )
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
Patch Set 7: Code-Review+2
--
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: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 25 Sep 2022 12:32:31 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3322 / KUDU-3319 HMS event logging
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18866 )
Change subject: [master] KUDU-3322 / KUDU-3319 HMS event logging
......................................................................
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc@892
PS5, Line 892: TestHMSDBErase
It seems this test fails with ASAN warning. Since it's not clear what's going on, we can skip the test in case of ASAN build, adding corresponding comment and TODO which we can address later on.
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc@914
PS5, Line 914: string str = "The event ID 2 last seen by Kudu master is greater "
: "than 0 currently reported by HMS. Has the HMS database "
: "been reset (backup&restore, etc.)?";
nit: since this is a constant string, maybe move this to the outer scope, adding 'const' or maybe even turning it into 'constexpr const char* const' ?
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/integration-tests/master_hms-itest.cc@919
PS5, Line 919: ASSERT_STR_CONTAINS(line, str);
Isn't this always true since it's under the 'if (line.find(str) != std::string::npos)' clause?
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/master/hms_notification_log_listener.cc
File src/kudu/master/hms_notification_log_listener.cc:
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/master/hms_notification_log_listener.cc@290
PS5, Line 290: events.empty()
Could you add a comment explaining why we are checking for that precondition only in case of events.empty() is true?
http://gerrit.cloudera.org:8080/#/c/18866/5/src/kudu/master/hms_notification_log_listener.cc@291
PS5, Line 291: int64_t event_id;
: RETURN_NOT_OK_PREPEND(catalog_manager_->hms_catalog()->
: GetCurrentNotificationEventId(&event_id),
: "failed to retrieve latest notification log event");
: if (event_id < processed_event_id) {
: LOG(ERROR) << Substitute("The event ID $0 last seen by Kudu master is greater "
: "than $1 currently reported by HMS. Has the HMS database "
: "been reset (backup&restore, etc.)?",
: processed_event_id, event_id);
: }
: }
Since the code under 'for()' clause doesn't do anything to 'events', maybe move this to be at line 239?
--
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: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 22 Sep 2022 20:21:26 +0000
Gerrit-HasComments: Yes