Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13073 )
Change subject: hms: skip drop table notification log for external table ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/13073/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13073/1//COMMIT_MSG@9 PS1, Line 9: synced. nit: could you keep the lines in this messages under 72 symbols wide? http://gerrit.cloudera.org:8080/#/c/13073/1//COMMIT_MSG@10 PS1, Line 10: external tables Do we have other cases besides Kudu+Impala integration where we have such 'external' semantic? If not, maybe change into 'external Kudu tables in Impala' ? Or the whole idea is that with this change HMS can support external Kudu tables as well? http://gerrit.cloudera.org:8080/#/c/13073/1//COMMIT_MSG@16 PS1, Line 16: tables nit: the table ? http://gerrit.cloudera.org:8080/#/c/13073/1/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/13073/1/src/kudu/integration-tests/master_hms-itest.cc@324 PS1, Line 324: ASSERT_OK(client_->OpenTable("default.e", &table)); Might it be any race here due to the nature of synchronization between Kudu and HMS metadata? Imagine HMS get a bug and sends table removal event to Kudu 5 seconds later. How do we know that's not the case here? -- To view, visit http://gerrit.cloudera.org:8080/13073 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ffb23d507cc0d9ba9e46983e5bbf6b7116f7515 Gerrit-Change-Number: 13073 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 24 Apr 2019 06:50:13 +0000 Gerrit-HasComments: Yes
