[kudu-CR] [test] Make the kudu-admin-test more rubust by adding AssertEventually
honeyhexin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14070 Change subject: [test] Make the kudu-admin-test more rubust by adding AssertEventually .. [test] Make the kudu-admin-test more rubust by adding AssertEventually Sometimes it's possible the scan will not immediately retrieve the row even though the write has already succeeded, therefore add AssertEventually to assure the scan return the right result. It will make the test case more robust. Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 38 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14070/1 -- To view, visit http://gerrit.cloudera.org:8080/14070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb Gerrit-Change-Number: 14070 Gerrit-PatchSet: 1 Gerrit-Owner: honeyhexin
[kudu-CR] docs: correct invalid link
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14028 ) Change subject: docs: correct invalid link .. Patch Set 1: I see. It looks like there is a correct way to handle this so it works in github and on the official site too. The Impala page has an example of this for the HMS integration: https://github.com/apache/kudu/blob/master/docs/kudu_impala_integration.adoc See <> for more details -- To view, visit http://gerrit.cloudera.org:8080/14028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I41e8671f518bb898dd357dc5024a7d8ab335b401 Gerrit-Change-Number: 14028 Gerrit-PatchSet: 1 Gerrit-Owner: XiaokaiWang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: XiaokaiWang Gerrit-Comment-Date: Thu, 15 Aug 2019 12:54:50 + Gerrit-HasComments: No
[kudu-CR] [tablet] Fixed the bug of DeltaTracker::CountDeletedRows
Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14061 ) Change subject: [tablet] Fixed the bug of DeltaTracker::CountDeletedRows .. Patch Set 4: I implemented an integration test case that simulated this scenario with high-frequency flush, tablet server crashes, and heartbeats(count live rows). -- To view, visit http://gerrit.cloudera.org:8080/14061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9bb4456123087778c9dc799777c5990938a84fdf Gerrit-Change-Number: 14061 Gerrit-PatchSet: 4 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 15 Aug 2019 15:00:35 + Gerrit-HasComments: No
[kudu-CR] [tablet] Fixed the bug of DeltaTracker::CountDeletedRows
Hello Kudu Jenkins, helifu, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14061 to look at the new patch set (#3). Change subject: [tablet] Fixed the bug of DeltaTracker::CountDeletedRows .. [tablet] Fixed the bug of DeltaTracker::CountDeletedRows When Tablet.CountLiveRows was called in a multi-thread case, there's a chance we'll see the following failure. User stack: F0814 12:05:51.975797 96375 diskrowset.cc:759] Check failed: *count >= 0 (-3 vs. 0) *** Check failure stack trace: *** *** Aborted at 156571 (unix time) try "date -d @156571" if you are using GNU date *** PC: @ 0x7f9bd20425f7 __GI_raise *** SIGABRT (@0x70900017872) received by PID 96370 (TID 0x7f9bce2d7700) from PID 96370; stack trace: *** @ 0x7f9bdaff6100 (unknown) @ 0x7f9bd20425f7 __GI_raise @ 0x7f9bd2043ce8 __GI_abort @ 0x7f9bd4540c99 google::logging_fail() @ 0x7f9bd454246d google::LogMessage::Fail() @ 0x7f9bd45443c3 google::LogMessage::SendToLog() @ 0x7f9bd4541fc9 google::LogMessage::Flush() @ 0x7f9bd4544d4f google::LogMessageFatal::~LogMessageFatal() @ 0x7f9bddc9aabe kudu::tablet::DiskRowSet::CountLiveRows() @ 0x7f9bddbdeb79 kudu::tablet::Tablet::CountLiveRows() @ 0x49891f kudu::tablet::MultiThreadedTabletTest<>::CollectStatisticsThread() @ 0x4ae34b boost::_mfi::mf1<>::operator()() @ 0x4add25 boost::_bi::list2<>::operator()<>() @ 0x4acfe9 boost::_bi::bind_t<>::operator()() @ 0x4ac8a6 boost::detail::function::void_function_obj_invoker0<>::invoke() @ 0x7f9bd7116492 boost::function0<>::operator()() @ 0x7f9bd62e5324 kudu::Thread::SuperviseThread() @ 0x7f9bdafeedc5 start_thread @ 0x7f9bd2103ced __clone This is because there is DeltaTracker lack of lock protection when modify the number of live rows in rowset_metadata_ and reset the deleted_row_count_. This caused deleted_row_count_ to be duplicated when calculating the number of live rows of DRS. Consider the following sequence: | T1| T2 |-- |-- |+ In DT::Flush | | Take compact_flush_lock_ (excl) | | Take component_lock_ (excl) | | deleted_row_count_ = ... | | Release component_lock_ | | + In DT::FlushDMS| |Call RSMD::IncrementLiveRows | |--> RSMD::live_row_count - deleted_row_count_ | |+ In DRS::CountLiveRows | | Take component_lock_ (shared) | | Call RSMD::live_row_count - DT::CountDeletedRows | | --> RSMD::live_row_count - deleted_row_count_ | | --> we double counted deleted_row_count_ !!! | Take component_lock_ (excl) | | deleted_row_count_ = 0 | | Release component_lock_ | | Release compact_flush_lock_ | Change-Id: I9bb4456123087778c9dc799777c5990938a84fdf --- M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/metadata-test.cc M src/kudu/tablet/mt-tablet-test.cc M src/kudu/tablet/rowset_metadata.cc M src/kudu/tablet/rowset_metadata.h 10 files changed, 148 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/14061/3 -- To view, visit http://gerrit.cloudera.org:8080/14061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9bb4456123087778c9dc799777c5990938a84fdf Gerrit-Change-Number: 14061 Gerrit-PatchSet: 3 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: helifu
[kudu-CR] [tablet] Fixed the bug of DeltaTracker::CountDeletedRows
Hello Tidy Bot, Kudu Jenkins, helifu, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14061 to look at the new patch set (#4). Change subject: [tablet] Fixed the bug of DeltaTracker::CountDeletedRows .. [tablet] Fixed the bug of DeltaTracker::CountDeletedRows When Tablet.CountLiveRows was called in a multi-thread case, there's a chance we'll see the following failure. User stack: F0814 12:05:51.975797 96375 diskrowset.cc:759] Check failed: *count >= 0 (-3 vs. 0) *** Check failure stack trace: *** *** Aborted at 156571 (unix time) try "date -d @156571" if you are using GNU date *** PC: @ 0x7f9bd20425f7 __GI_raise *** SIGABRT (@0x70900017872) received by PID 96370 (TID 0x7f9bce2d7700) from PID 96370; stack trace: *** @ 0x7f9bdaff6100 (unknown) @ 0x7f9bd20425f7 __GI_raise @ 0x7f9bd2043ce8 __GI_abort @ 0x7f9bd4540c99 google::logging_fail() @ 0x7f9bd454246d google::LogMessage::Fail() @ 0x7f9bd45443c3 google::LogMessage::SendToLog() @ 0x7f9bd4541fc9 google::LogMessage::Flush() @ 0x7f9bd4544d4f google::LogMessageFatal::~LogMessageFatal() @ 0x7f9bddc9aabe kudu::tablet::DiskRowSet::CountLiveRows() @ 0x7f9bddbdeb79 kudu::tablet::Tablet::CountLiveRows() @ 0x49891f kudu::tablet::MultiThreadedTabletTest<>::CollectStatisticsThread() @ 0x4ae34b boost::_mfi::mf1<>::operator()() @ 0x4add25 boost::_bi::list2<>::operator()<>() @ 0x4acfe9 boost::_bi::bind_t<>::operator()() @ 0x4ac8a6 boost::detail::function::void_function_obj_invoker0<>::invoke() @ 0x7f9bd7116492 boost::function0<>::operator()() @ 0x7f9bd62e5324 kudu::Thread::SuperviseThread() @ 0x7f9bdafeedc5 start_thread @ 0x7f9bd2103ced __clone This is because there is DeltaTracker lack of lock protection when modify the number of live rows in rowset_metadata_ and reset the deleted_row_count_. This caused deleted_row_count_ to be duplicated when calculating the number of live rows of DRS. Consider the following sequence: | T1| T2 |-- |-- |+ In DT::Flush | | Take compact_flush_lock_ (excl) | | Take component_lock_ (excl) | | deleted_row_count_ = ... | | Release component_lock_ | | + In DT::FlushDMS| |Call RSMD::IncrementLiveRows | |--> RSMD::live_row_count - deleted_row_count_ | |+ In DRS::CountLiveRows | | Take component_lock_ (shared) | | Call RSMD::live_row_count - DT::CountDeletedRows | | --> RSMD::live_row_count - deleted_row_count_ | | --> we double counted deleted_row_count_ !!! | Take component_lock_ (excl) | | deleted_row_count_ = 0 | | Release component_lock_ | | Release compact_flush_lock_ | Change-Id: I9bb4456123087778c9dc799777c5990938a84fdf --- M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/metadata-test.cc M src/kudu/tablet/mt-tablet-test.cc M src/kudu/tablet/rowset_metadata.cc M src/kudu/tablet/rowset_metadata.h 10 files changed, 145 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/14061/4 -- To view, visit http://gerrit.cloudera.org:8080/14061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9bb4456123087778c9dc799777c5990938a84fdf Gerrit-Change-Number: 14061 Gerrit-PatchSet: 4 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: helifu
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 7:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h
File src/kudu/client/schema.h:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h@583
PS6, Line 583: bool HasColumn(const std::string& col_name, KuduColumnSchema*
col_schema) const;
> Maybe rename to "HasColumn"?
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h@583
PS6, Line 583: e, KuduColumnSchem
> Nit: "KuduColumnSchema* "
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@18
PS6, Line 18: #include
> Nit: prefer cstdlib over this.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@86
PS6, Line 86: "Also check for the existence of the row on the
leader replica of "
: "the tablet. If found, the full row will be
> Good question. We have implemented "sub-modes" (i.e. mode within another mo
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@676
PS6, Line 676: Status DropRangePartition(const RunnerContext& context) {
> Don't need std:: prefix for std::string.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@677
PS6, Line 677: return ModifyRangePartition(context, PartitionAction::DROP);
> Could this be defined statically and using an initializer_list:
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@695
PS6, Line 695: "Only one default value should be provided, but we got $0
value",
: std::to_string(values.size(;
> Nit: indentation
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@698
PS6, Line 698: switch (type) {
> But for STRING/BINARY columns, isn't the empty string a valid value?
If users want to set default value of a STRING/BINARY column to an empty string
, they could remove_default for the column.
I finally set the default_value argument as a JSON array on account of another
problem. That is if we want to set_default for a INT column to a negative
number, such as '-1', the command line tool will parse it as a flag.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@706
PS6, Line 706: reader.ExtractInt64(values[0], /*field=*/nullptr,
_value),
: Substitute("unable to parse value for column type $0",
: KuduColumnSchema::DataTypeToString(type)));
: *value = KuduValue::FromInt(int_value);
: b
> Use safe_strto64() here instead. And use the other safe_ functions below in
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@716
PS6, Line 716: reader.ExtractString(values[0], /*field=*/nullptr,
_value),
> Isn't this guaranteed by L697?
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@720
PS6, Line 720: break;
: }
: case KuduColumnSchema::DataType::BOOL: {
: bool bool_value;
: RETURN_NOT_OK_PREPEND(
> Use boost::iequals() for the comparison to make it case-insensitive.
Here I parse it as a JSON value.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@736
PS6, Line 736:KuduColumnSchema::DataTypeToString(type)));
> Shouldn't this be DataType::DECIMAL? Same above for UNIXTIME_MICROS.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@737
PS6, Line 737: *value = KuduValue::FromFloat(double_value);
: break;
: }
: case KuduColumnSchema::DataType::DOUBLE: {
: double double_value;
: RETURN_NOT_OK_PREPEND(
: reader.ExtractDouble(values[0], /*field=*/n
> These can be combined.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@750
PS6, Line 750: default:
> Same comment about static definition and an initializer_list.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@759
PS6, Line 759: Status ColumnSetDefault(const RunnerContext& context) {
: const string& table_name = FindOrDie(context.required_args,
kTableNameArg);
> This is duplicated.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@779
PS6, Line 779:
> Same.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@1021
PS6,
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#7). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: 1. kudu table column_set_default should be provided as a JSON array, e.g. [1] or ["foo"]. 2. kudu table column_remove_default 3. kudu table column_set_compression 4. kudu table column_set_encoding 5. kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 532 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/7 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 7 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [metrics] Merge metrics by the same attribute
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13580 to look at the new patch set (#11). Change subject: [metrics] Merge metrics by the same attribute .. [metrics] Merge metrics by the same attribute This patch merge metrics together which have the same value of some attributes, in order to reduce the total data size received by any thirdparty monitor systems if they do not care about the original metrics details. For example, fetch metrics from tserver by: http://:/metrics?merge_rules=tablet|table|table_name All 'tablet' type metrics which have the same value of 'table_name' attribute, will be merged together into a new 'table' type metrics, and metric values will be aggregated. Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 --- M src/kudu/server/default_path_handlers.cc M src/kudu/util/hdr_histogram-test.cc M src/kudu/util/hdr_histogram.cc M src/kudu/util/hdr_histogram.h M src/kudu/util/metrics-test.cc M src/kudu/util/metrics.cc M src/kudu/util/metrics.h 7 files changed, 876 insertions(+), 178 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/13580/11 -- To view, visit http://gerrit.cloudera.org:8080/13580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 Gerrit-Change-Number: 13580 Gerrit-PatchSet: 11 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#8). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: 1. kudu table column_set_default should be provided as a JSON array, e.g. [1] or ["foo"]. 2. kudu table column_remove_default 3. kudu table column_set_compression 4. kudu table column_set_encoding 5. kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 531 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/8 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 8 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [metrics] Merge metrics by the same attribute
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13580 to look at the new patch set (#12). Change subject: [metrics] Merge metrics by the same attribute .. [metrics] Merge metrics by the same attribute This patch merge metrics together which have the same value of some attributes, in order to reduce the total data size received by any thirdparty monitor systems if they do not care about the original metrics details. For example, fetch metrics from tserver by: http://:/metrics?merge_rules=tablet|table|table_name All 'tablet' type metrics which have the same value of 'table_name' attribute, will be merged together into a new 'table' type metrics, and metric values will be aggregated. Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 --- M src/kudu/server/default_path_handlers.cc M src/kudu/util/hdr_histogram-test.cc M src/kudu/util/hdr_histogram.cc M src/kudu/util/hdr_histogram.h M src/kudu/util/metrics-test.cc M src/kudu/util/metrics.cc M src/kudu/util/metrics.h 7 files changed, 876 insertions(+), 178 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/13580/12 -- To view, visit http://gerrit.cloudera.org:8080/13580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 Gerrit-Change-Number: 13580 Gerrit-PatchSet: 12 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [metrics] Merge metrics by the same attribute
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13580 ) Change subject: [metrics] Merge metrics by the same attribute .. Patch Set 11: (4 comments) http://gerrit.cloudera.org:8080/#/c/13580/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13580/10//COMMIT_MSG@14 PS10, Line 14: http://:/metrics?merge_rules=tablet|table|table_name > Should probably surround table_name with triangular brackets (i.e. http://gerrit.cloudera.org:8080/#/c/13580/10/src/kudu/util/hdr_histogram.h File src/kudu/util/hdr_histogram.h: http://gerrit.cloudera.org:8080/#/c/13580/10/src/kudu/util/hdr_histogram.h@174 PS10, Line 174: Merges 'other' into this HdrHistogram > Nit: "Merges 'other' into this HdrHistogram. Values in each..." Done http://gerrit.cloudera.org:8080/#/c/13580/10/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/13580/10/src/kudu/util/metrics.h@744 PS10, Line 744: // Merges 'other' into this Metric > Nit: "Merges 'other' into this Metric object." Done http://gerrit.cloudera.org:8080/#/c/13580/10/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/13580/10/src/kudu/util/metrics.cc@71 PS10, Line 71: JsonWriter* > Nit: "JsonWriter* " Done -- To view, visit http://gerrit.cloudera.org:8080/13580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 Gerrit-Change-Number: 13580 Gerrit-PatchSet: 11 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 15 Aug 2019 09:59:52 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#9). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: 1. kudu table column_set_default should be provided as a JSON array, e.g. [1] or ["foo"]. 2. kudu table column_remove_default 3. kudu table column_set_compression 4. kudu table column_set_encoding 5. kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 531 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/9 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 9 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-1938 [java] Add support for CHAR/VARCHAR pt 4
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/14050 )
Change subject: KUDU-1938 [java] Add support for CHAR/VARCHAR pt 4
..
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14050/2/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/14050/2/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@121
PS2, Line 121: if (type == Type.CHAR) {
: if (typeAttributes == null || !typeAttributes.hasLength()
|| typeAttributes.getLength() < CharUtil.MIN_CHAR_LENGTH
: || typeAttributes.getLength() >
CharUtil.MAX_CHAR_LENGTH) {
: throw new IllegalArgumentException(
: String.format("CHAR's length must be set and between
%d and %d", CharUtil.MIN_CHAR_LENGTH,
: CharUtil.MAX_CHAR_LENGTH));
: }
: }
:
: if (type == Type.VARCHAR) {
: if (typeAttributes == null || !typeAttributes.hasLength()
|| typeAttributes.getLength() < CharUtil.MIN_VARCHAR_LENGTH
: || typeAttributes.getLength() >
CharUtil.MAX_VARCHAR_LENGTH) {
: throw new IllegalArgumentException(
: String.format("VARCHAR's length must be set and
between %d and %d", CharUtil.MIN_VARCHAR_LENGTH,
: CharUtil.MAX_VARCHAR_LENGTH));
: }
: }
> > I think validation like this should be in the builders build() call and n
hmm, I can't remember if we do anything special for Decimal. I thought the
attributes were validated server side. Maybe not.
http://gerrit.cloudera.org:8080/#/c/14050/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14050/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@642
PS2, Line 642:* Add a CHAR for the specified column.
> Added them. RowResult doesn't do any truncation/padding, should I point out
I think with the name change it's clear enough.
http://gerrit.cloudera.org:8080/#/c/14050/4/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14050/4/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@692
PS4, Line 692: public void addChar(int columnIndex, byte[] val) {
> KeyEncoder uses them. I still need to convert these byte[]s to Strings for
If KeyEncoder can use the string version I think that makes sense to convert to
a string there. It's a quick oneline wrapper. So adding the API here seems
overkill.
If you decide to keep these, if they only exist for KeyEncoder can they be
package-private and not a part of the public api?
http://gerrit.cloudera.org:8080/#/c/14050/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14050/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@762
PS7, Line 762: public String getString(String columnName) {
Is it technically correct to substring by characters while the string is still
UTF-16 (java default)? Or should we convert to bytes and then truncate the
extra bytes?
http://gerrit.cloudera.org:8080/#/c/14050/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@764
PS7, Line 764: }
What happens in the case where a CHAR column has trailing spaces to so that
length is >= val.length? Should we move this above that if statement?
http://gerrit.cloudera.org:8080/#/c/14050/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@769
PS7, Line 769:* @return a string
This else statement is the same as the end the if statement. Can this be
re-worked to reuse the end logic?
--
To view, visit http://gerrit.cloudera.org:8080/14050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03edf5e65409e895512d5cd81a607180632e8995
Gerrit-Change-Number: 14050
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 15 Aug 2019 16:53:49 +
Gerrit-HasComments: Yes
[kudu-CR] [docs] updated docs w.r.t. collocation practices
Hello Will Berkeley, Alex Rodoni, Kudu Jenkins, Mitch Barnett, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12997 to look at the new patch set (#5). Change subject: [docs] updated docs w.r.t. collocation practices .. [docs] updated docs w.r.t. collocation practices The administration.adoc has been updated to discourage collocating of Kudu tablet servers with masters and other tablet servers. Also, fixed few misprints. Change-Id: I88a38117751cfa436c1fd95598274fb8f01f04ea --- M docs/administration.adoc 1 file changed, 37 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/12997/5 -- To view, visit http://gerrit.cloudera.org:8080/12997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88a38117751cfa436c1fd95598274fb8f01f04ea Gerrit-Change-Number: 12997 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tools] Add table tools to delete column and alter column
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 9:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@698
PS6, Line 698: case KuduColumnSchema::DataType::INT8:
> If users want to set default value of a STRING/BINARY column to an empty st
Just to clarify: with the new JSON encoding, empty strings are treated as valid
default values just like any other string, right? Could you verify that with a
unit test?
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@173
PS9, Line 173: const unordered_map kCompressionTypeMap {
We try to avoid declaring global non-POD-type variables. They're not so much an
issue in executables, but in libraries they affect loading performance and
their initialization order is undefined (which is a problem when one global
depends on another).
Instead, declare these as static variables within whichever function needs them.
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@706
PS9, Line 706: Substitute("unable to parse value for column type $0",
: KuduColumnSchema::DataTypeToString(type))
Maybe you can declare this up front so you needn't duplicate it in multiple
case statements.
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@767
PS9, Line 767: KuduColumnSchema col_schema = schema.Column(0);
Could you add a comment here explaining that we're not actually interested in
the first column of the schema, but that there's no default constructor for
KuduColumnSchema so we're forced to create one using some valid column?
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@797
PS9, Line 797: auto it = kCompressionTypeMap.find(compression_type_uc);
: if (it == kCompressionTypeMap.end()) {
Should be able to use one of the FindOr* functions from gutil/map-util.h here
(and in ColumnSetEncoding).
--
To view, visit http://gerrit.cloudera.org:8080/13976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f
Gerrit-Change-Number: 13976
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Thu, 15 Aug 2019 16:50:56 +
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Merge metrics by the same attribute
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13580 ) Change subject: [metrics] Merge metrics by the same attribute .. Patch Set 12: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13580/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13580/10//COMMIT_MSG@14 PS10, Line 14: http://:/metrics?merge_rules=tablet|table|table_name > 'table_name' here is itself, not a table name value, it indicate to the 'ta Ah ok. -- To view, visit http://gerrit.cloudera.org:8080/13580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 Gerrit-Change-Number: 13580 Gerrit-PatchSet: 12 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 15 Aug 2019 16:30:13 + Gerrit-HasComments: Yes
[kudu-CR] [metrics] Merge metrics by the same attribute
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13580 ) Change subject: [metrics] Merge metrics by the same attribute .. [metrics] Merge metrics by the same attribute This patch merge metrics together which have the same value of some attributes, in order to reduce the total data size received by any thirdparty monitor systems if they do not care about the original metrics details. For example, fetch metrics from tserver by: http://:/metrics?merge_rules=tablet|table|table_name All 'tablet' type metrics which have the same value of 'table_name' attribute, will be merged together into a new 'table' type metrics, and metric values will be aggregated. Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 Reviewed-on: http://gerrit.cloudera.org:8080/13580 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/server/default_path_handlers.cc M src/kudu/util/hdr_histogram-test.cc M src/kudu/util/hdr_histogram.cc M src/kudu/util/hdr_histogram.h M src/kudu/util/metrics-test.cc M src/kudu/util/metrics.cc M src/kudu/util/metrics.h 7 files changed, 876 insertions(+), 178 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8db3d082ae847eb1d83b9e4aee57d5e4bf13e1b5 Gerrit-Change-Number: 13580 Gerrit-PatchSet: 13 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [test] Make the kudu-admin-test more rubust by adding AssertEventually
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14070 ) Change subject: [test] Make the kudu-admin-test more rubust by adding AssertEventually .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG@9 PS1, Line 9: Sometimes it's possible the scan will not immediately retrieve the row : even though the write has already succeeded, therefore add AssertEventually : to assure the scan return the right result. It will make the test case more robust. > Agreed when the scan follows a drop range partition, but we don't need it e +1 Since CountTableRows() use LEADER_ONLY scan mode and the prior write operation succeeds, the rows should be there. Adding ASSERT_EVENTUALLY in such cases would mask a bug (if any appears). -- To view, visit http://gerrit.cloudera.org:8080/14070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb Gerrit-Change-Number: 14070 Gerrit-PatchSet: 1 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 15 Aug 2019 16:40:10 + Gerrit-HasComments: Yes
[kudu-CR] [test] Make the kudu-admin-test more rubust by adding AssertEventually
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/14070 )
Change subject: [test] Make the kudu-admin-test more rubust by adding
AssertEventually
..
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG@7
PS1, Line 7: rubust
robust
http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG@9
PS1, Line 9: Sometimes it's possible the scan will not immediately retrieve the
row
: even though the write has already succeeded, therefore add
AssertEventually
: to assure the scan return the right result. It will make the test
case more robust.
Agreed when the scan follows a drop range partition, but we don't need it
everywhere. For example, after a bunch of inserts with a flush, the rows are
guaranteed to be read by the next scan.
I've left comments in the code with the locations of AssertEventually calls
that we don't need.
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2394
PS1, Line 2394: ASSERT_EVENTUALLY([&]() {
Or here.
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2434
PS1, Line 2434: ASSERT_EVENTUALLY([&]() {
Or here.
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2535
PS1, Line 2535: ASSERT_EVENTUALLY([&]() {
Or here.
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2564
PS1, Line 2564: ASSERT_EVENTUALLY([&]() {
Or here.
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2847
PS1, Line 2847: ASSERT_EVENTUALLY([&]() {
Don't need it here.
--
To view, visit http://gerrit.cloudera.org:8080/14070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb
Gerrit-Change-Number: 14070
Gerrit-PatchSet: 1
Gerrit-Owner: honeyhexin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 15 Aug 2019 16:34:05 +
Gerrit-HasComments: Yes
[kudu-CR] [docs] updated docs w.r.t. collocation practices
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12997 ) Change subject: [docs] updated docs w.r.t. collocation practices .. Patch Set 4: > What is the status of this? Is this abandoned? Whoops, it seems I forgot to revv it last time. I updated the wording to discourage the master-tserver collocation. -- To view, visit http://gerrit.cloudera.org:8080/12997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88a38117751cfa436c1fd95598274fb8f01f04ea Gerrit-Change-Number: 12997 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 15 Aug 2019 17:09:28 + Gerrit-HasComments: No
[kudu-CR] POC: Disable Sentry related tests
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14020 ) Change subject: POC: Disable Sentry related tests .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14020/1/src/kudu/integration-tests/CMakeLists.txt File src/kudu/integration-tests/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/14020/1/src/kudu/integration-tests/CMakeLists.txt@93 PS1, Line 93: # NOTE: Sentry tests are disabled to allow upgrading to Hive 3. : #ADD_KUDU_TEST(master_sentry-itest RUN_SERIAL true NUM_SHARDS 8 PROCESSORS 4) > Less bit rot. It's more frustrating to fix bit rot at test compile _and_ ru I didn't see a simple way to do this without a bunch of line changes. -- To view, visit http://gerrit.cloudera.org:8080/14020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b99b0de221f6d4acfb427f830cd3344c8d0a10b Gerrit-Change-Number: 14020 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 15 Aug 2019 19:53:01 + Gerrit-HasComments: Yes
[kudu-CR] [test] Make the kudu-admin-test more robust by adding AssertEventually
honeyhexin has posted comments on this change. (
http://gerrit.cloudera.org:8080/14070 )
Change subject: [test] Make the kudu-admin-test more robust by adding
AssertEventually
..
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG@7
PS1, Line 7: robust
> robust
Done
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2394
PS1, Line 2394: ASSERT_OK(client_->OpenTable(kTableId, ));
> Or here.
Done
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2434
PS1, Line 2434: }
> Or here.
Done
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2535
PS1, Line 2535: // which will return error in lambda as we expect.
> Or here.
Done
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2564
PS1, Line 2564: ASSERT_EVENTUALLY([&]() {
> Or here.
Done
http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2847
PS1, Line 2847: "drop_range_partition",
> Don't need it here.
Oops, I made a mistake. I will fix it.
--
To view, visit http://gerrit.cloudera.org:8080/14070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb
Gerrit-Change-Number: 14070
Gerrit-PatchSet: 2
Gerrit-Owner: honeyhexin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: honeyhexin
Gerrit-Comment-Date: Thu, 15 Aug 2019 22:25:05 +
Gerrit-HasComments: Yes
[kudu-CR] [test] Make the kudu-admin-test more robust by adding AssertEventually
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14070 to look at the new patch set (#3). Change subject: [test] Make the kudu-admin-test more robust by adding AssertEventually .. [test] Make the kudu-admin-test more robust by adding AssertEventually Sometimes it's possible the scan will not immediately retrieve the row even though the write has already succeeded, therefore add AssertEventually to assure the scan return the right result. It will make the test case more robust. Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 18 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14070/3 -- To view, visit http://gerrit.cloudera.org:8080/14070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb Gerrit-Change-Number: 14070 Gerrit-PatchSet: 3 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: honeyhexin
[kudu-CR] [test] Make the kudu-admin-test more robust by adding AssertEventually
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14070 ) Change subject: [test] Make the kudu-admin-test more robust by adding AssertEventually .. [test] Make the kudu-admin-test more robust by adding AssertEventually Sometimes it's possible the scan will not immediately retrieve the row even though the write has already succeeded, therefore add AssertEventually to assure the scan return the right result. It will make the test case more robust. Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb Reviewed-on: http://gerrit.cloudera.org:8080/14070 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 18 insertions(+), 9 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/14070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb Gerrit-Change-Number: 14070 Gerrit-PatchSet: 4 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: honeyhexin
[kudu-CR] [test] Make the kudu-admin-test more robust by adding AssertEventually
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14070 ) Change subject: [test] Make the kudu-admin-test more robust by adding AssertEventually .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb Gerrit-Change-Number: 14070 Gerrit-PatchSet: 3 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Thu, 15 Aug 2019 23:54:41 + Gerrit-HasComments: No
[kudu-CR] [test] Make the kudu-admin-test more robust by adding AssertEventually
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14070 to look at the new patch set (#2). Change subject: [test] Make the kudu-admin-test more robust by adding AssertEventually .. [test] Make the kudu-admin-test more robust by adding AssertEventually Sometimes it's possible the scan will not immediately retrieve the row even though the write has already succeeded, therefore add AssertEventually to assure the scan return the right result. It will make the test case more robust. Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 18 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14070/2 -- To view, visit http://gerrit.cloudera.org:8080/14070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb Gerrit-Change-Number: 14070 Gerrit-PatchSet: 2 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Prepare for upgrading to Hive 3
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/14018 )
Change subject: Prepare for upgrading to Hive 3
..
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14018/5/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/14018/5/src/kudu/hms/mini_hms.cc@156
PS5, Line 156: { "HADOOP_CONF_DIR", data_root_ },
Is the a duplicate for line 152?
http://gerrit.cloudera.org:8080/#/c/14018/5/src/kudu/hms/mini_hms.cc@158
PS5, Line 158: TODO(ghenke)
nit: maybe, replace with
TODO(HADOOP-15966): remove after the issue is addressed (Hadoop 3.1.3+)
?
--
To view, visit http://gerrit.cloudera.org:8080/14018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If43ae2330b3d99374c68bae313a3f8bc070f9c69
Gerrit-Change-Number: 14018
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 15 Aug 2019 20:01:23 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2212 mute TSAN on destruction of a locked mutex in glog
Hello Kudu Jenkins, Andrew Wong, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13763 to look at the new patch set (#2). Change subject: KUDU-2212 mute TSAN on destruction of a locked mutex in glog .. KUDU-2212 mute TSAN on destruction of a locked mutex in glog This patch adds a suppression for TSAN warning about destruction of a locked mutex in libglog. Since the only registered instances of the issue happen only during shutdown of the C++ runtime upon exit of kudu CLI process, it seems safe to mute it for a while. In addition, I added a suppression for a warning about possible deadlock while shutting down libglog's runtime. The motivation for this change is to hunt down another issue tracked by KUDU-2878: while doing so, this benign TSAN warning hinders the scenario used to RCA the latter. The suppression of the warnings for a locked mutex relies on the fix for issue 944 in the thread sanitizers. The fix is back-ported as a patch for LLVM 6.0 in Kudu's thirdparty. For details on the TSAN suppressions fix, see https://github.com/google/sanitizers/issues/944. The results of two representative 1K runs are below: kudu-admin-test built in TSAN with --gtest_filter='*ToolTest.TestDeleteTable*' before and after the fix. before 2 out of 1K failed: http://dist-test.cloudera.org/job?job_id=aserbin.1565930436.112333 after 0 out of 1K failed: http://dist-test.cloudera.org/job?job_id=aserbin.1565931149.115149 Change-Id: If0b62027a1a4db18bfaa6c89b3070c918fc4f88a --- M src/kudu/util/sanitizer_options.cc 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/63/13763/2 -- To view, visit http://gerrit.cloudera.org:8080/13763 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If0b62027a1a4db18bfaa6c89b3070c918fc4f88a Gerrit-Change-Number: 13763 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [llvm] back-port of fix for TSAN issue 944
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14078 Change subject: [llvm] back-port of fix for TSAN issue 944 .. [llvm] back-port of fix for TSAN issue 944 See https://github.com/google/sanitizers/issues/944 for details. Change-Id: I3992b697201ba8274a3e7ae7171c7855ed7f5e71 --- M thirdparty/download-thirdparty.sh A thirdparty/patches/llvm-fix-944-destruction-of-a-locked-mutex.patch 2 files changed, 120 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/14078/1 -- To view, visit http://gerrit.cloudera.org:8080/14078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3992b697201ba8274a3e7ae7171c7855ed7f5e71 Gerrit-Change-Number: 14078 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
