[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Dan Burkert has submitted this change and it was merged. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Reviewed-on: http://gerrit.cloudera.org:8080/3255 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 610 insertions(+), 317 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Adar Dembo has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 9: Build Started http://104.196.14.100/job/kudu-gerrit/1740/ -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#9). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 610 insertions(+), 317 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/9 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Dan Burkert has posted comments on this change.
Change subject: KUDU-1308 [c++-client]: support tables with non-covering range
partitions
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3255/4/src/kudu/integration-tests/flex_partitioning-itest.cc
File src/kudu/integration-tests/flex_partitioning-itest.cc:
PS4, Line 185: // Tablets aren't always immediately dropped, spin until they
are gone.
: for (int i = 1; CountTablets() > 0; i++) {
: CHECK_LE(i, 30);
: base::SleepForMilliseconds(10 * i);
: }
> Why is this important for the correctness of these tests?
Done
PS4, Line 250:
CHECK_OK(itest::CreateTabletServerMap(cluster_->master_proxy().get(),
: cluster_->messenger(),
: &ts_map));
:
: vector
tablets;
: CHECK_OK(ListTablets(ts_map.begin()->second,
MonoDelta::FromSeconds(10), &tablets));
: return tablets.size();
> It's a method on MasterServiceProxy.
Done
--
To view, visit http://gerrit.cloudera.org:8080/3255
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/1738/ -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#8). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 609 insertions(+), 317 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/8 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Adar Dembo has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/3255/4/src/kudu/integration-tests/flex_partitioning-itest.cc File src/kudu/integration-tests/flex_partitioning-itest.cc: PS4, Line 250: CHECK_OK(itest::CreateTabletServerMap(cluster_->master_proxy().get(), : cluster_->messenger(), : &ts_map)); : : vector tablets; : CHECK_OK(ListTablets(ts_map.begin()->second, MonoDelta::FromSeconds(10), &tablets)); : return tablets.size(); > What class is GetTableLocations defined on? It's a method on MasterServiceProxy. -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Dan Burkert has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: PS2, Line 1061: ASSERT_NO_FATAL_FAILURE(InsertTestRows(table.get(), 50, 0)); : client_->data_->meta_cache_->ClearCacheForTesting(); : ASSERT_NO_FATAL_FAILURE(InsertTestRows(table.get(), 50, 50)); : client_->data_->meta_cache_->ClearCacheForTesting(); : ASSERT_NO_FATAL_FAILURE(InsertTestRows(table.get(), 100, 200)); : client_->data_->meta_cache_->ClearCacheForTesting(); > What's the significance of clearing the cache? Add a comment? Done Line 1068: // Insert an out-of-range rows > Nit: no need for 'an'. Done Line 1084: Status result = session->Flush(); > I think you missed this comment. Done -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Dan Burkert has posted comments on this change.
Change subject: KUDU-1308 [c++-client]: support tables with non-covering range
partitions
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:
PS2, Line 308: else if (!s.ok()) {
: return s;
: }
> You don't need the 'else' statement either.
Done
--
To view, visit http://gerrit.cloudera.org:8080/3255
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/1726/ -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#7). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 597 insertions(+), 314 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/7 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#6). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 597 insertions(+), 314 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/6 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/1721/ -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#5). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 580 insertions(+), 298 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/5 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Dan Burkert has posted comments on this change.
Change subject: KUDU-1308 [c++-client]: support tables with non-covering range
partitions
..
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 811: void MetaCache::ClearCacheForTesting() {
> Surprised ASAN didn't complain about this. More missing test coverage?
There are ASAN failures, but I haven't tracked them all down yet...
http://gerrit.cloudera.org:8080/#/c/3255/4/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
PS4, Line 776: if (not_found) return Status::NotFound("No tablet covering the
requested range partition");
: return Status::OK();
> Nit: how about:
Done
http://gerrit.cloudera.org:8080/#/c/3255/4/src/kudu/integration-tests/flex_partitioning-itest.cc
File src/kudu/integration-tests/flex_partitioning-itest.cc:
Line 63: HashPartitionOptions(vector columns,
> Do you really need the constructors for this and RangePartitionOptions? Can
Didn't know that was an option. TIL.
Line 249: STLDeleteValues(&ts_map);
> What's this doing here? The map is new.
Done
PS4, Line 250:
CHECK_OK(itest::CreateTabletServerMap(cluster_->master_proxy().get(),
: cluster_->messenger(),
: &ts_map));
:
: vector
tablets;
: CHECK_OK(ListTablets(ts_map.begin()->second,
MonoDelta::FromSeconds(10), &tablets));
: return tablets.size();
> Seems strange to do this via a tserver call instead of GetTableLocations()
What class is GetTableLocations defined on?
Line 263: Status InsertRows(const RangePartitionOptions& range_partition,
int* row_count);
> Nit: also add that on success, row_count contains the number of rows actual
Done
Line 297: vector, vector>> default_bounds = { {
{ 0 }, { 1000 } } };
> Maybe should be static and declared as kDefaultBounds?
Done
Line 552: NO_FATALS(TestPartitionOptions(hash_option, range_option));
> Does this do the right thing even though there are no NO_FATALS() wrappers
I'm not sure. I made sure every call site of a helper function that has an
assert is wrapped in NO_FATALS, but maybe this isn't the correct way
--
To view, visit http://gerrit.cloudera.org:8080/3255
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/1719/ -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Adar Dembo has posted comments on this change.
Change subject: KUDU-1308 [c++-client]: support tables with non-covering range
partitions
..
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/3255/4/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 776: if (not_found) return Status::NotFound("No tablet covering the
requested range partition");
: return Status::OK();
Nit: how about:
return not_found ? Status::NotFound("...") : Status::OK();
http://gerrit.cloudera.org:8080/#/c/3255/4/src/kudu/integration-tests/flex_partitioning-itest.cc
File src/kudu/integration-tests/flex_partitioning-itest.cc:
Line 63: HashPartitionOptions(vector columns,
Do you really need the constructors for this and RangePartitionOptions? Can't
get by with regular C++ struct initialization? e.g.
HashPartitionOptions opts = { {"foo"}, 1 };
Line 185: // Tablets aren't always immediately dropped, spin until they are
gone.
: for (int i = 1; CountTablets() > 0; i++) {
: CHECK_LE(i, 30);
: base::SleepForMilliseconds(10 * i);
: }
Why is this important for the correctness of these tests?
Line 249: STLDeleteValues(&ts_map);
What's this doing here? The map is new.
Line 250: CHECK_OK(itest::CreateTabletServerMap(cluster_->master_proxy().get(),
: cluster_->messenger(),
: &ts_map));
:
: vector tablets;
: CHECK_OK(ListTablets(ts_map.begin()->second,
MonoDelta::FromSeconds(10), &tablets));
: return tablets.size();
Seems strange to do this via a tserver call instead of GetTableLocations() with
no bounds to the master. If you do the latter, I bet you won't have to wait for
tablets to actually die in DeleteTable() either.
Line 263: Status InsertRows(const RangePartitionOptions& range_partition,
int* row_count);
Nit: also add that on success, row_count contains the number of rows actually
written.
Line 297: vector, vector>> default_bounds = { {
{ 0 }, { 1000 } } };
Maybe should be static and declared as kDefaultBounds?
Line 552: NO_FATALS(TestPartitionOptions(hash_option, range_option));
Does this do the right thing even though there are no NO_FATALS() wrappers
within TestPartitionOptions itself?
--
To view, visit http://gerrit.cloudera.org:8080/3255
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Adar Dembo has posted comments on this change.
Change subject: KUDU-1308 [c++-client]: support tables with non-covering range
partitions
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
Line 140: vector required_feature_flags) {
> oops, forgot the last plumbing step.
Kind of surprised nothing failed without this. Are we lacking in some test
coverage?
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 1084: Status result = session->Flush();
> If you're going to flush with each row, how about using KuduSession::AUTO_F
I think you missed this comment.
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 811: ts_cache_.clear();
> Done
Surprised ASAN didn't complain about this. More missing test coverage?
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:
Line 308: else if (!s.ok()) {
: return s;
: }
> Done
You don't need the 'else' statement either.
--
To view, visit http://gerrit.cloudera.org:8080/3255
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Dan Burkert has posted comments on this change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: Line 1045: KuduPartialRow* a_lower_bound = schema_.NewRow(); > We already leak the split rows in the same failure scenario, so I don't thi took a stab at fixing this in http://gerrit.cloudera.org:8080/#/c/3275/ -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Dan Burkert has posted comments on this change.
Change subject: KUDU-1308 [c++-client]: support tables with non-covering range
partitions
..
Patch Set 2:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
Line 140: vector required_feature_flags) {
> This is never used?
oops, forgot the last plumbing step.
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 383: // Creates a table with 'num_replicas', split into tablets based on
'split_rows'
: // (or single tablet if 'split_rows' is empty).
> Update this comment.
Done
Line 1045: KuduPartialRow* a_lower_bound = schema_.NewRow();
> Nit: to be completely safe, we ought to store these in unique_ptrs, then re
We already leak the split rows in the same failure scenario, so I don't think
it's worth doing. Pretty unfortunate, as it's a strong indicator that our
client API is too difficult to use correctly. If I could do it over again, I
would change
KuduTableCreator& split_rows(const std::vector&
split_rows);
to
KuduTableCreator& add_split_row(KuduPartialRow* split_row);
Line 1140: {
> Nit: one-line comments for these test cases too?
Done
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 368: Add a partition range bound to the table with an inclusive lower
bound and
: // exclusive upper bound.
> I thought we were going to offer inclusive upper bounds? That's what I reme
We can, as another method. I would prefer to leave that to a follow-up commit.
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 755: if (tablets_by_key.empty()) {
> Should we do this before upper_bound(), since it doesn't depend on its resu
yes, but because I removed the duplicate Status constructors it ends up being
easier to keep it. If the collection is empty the upper_bound call is a no-op.
Line 756: return Status::NotFound("No tablet covering the requested range
partition");
> Nit: you're creating the same Status in three different places. Perhaps use
Done
Line 758: // The requested partition key is before all tablets
> Nit: end with period. Do the same for the others here, please.
Done
Line 762: *remote_tablet = itr->second;
> Is the dereference on the RHS safe? Does itr == begin() imply that there's
It does in this case, since we already checked that the collection is non-empty.
Line 763: std::prev(itr)->second->partition().partition_key_end() >
rpc.partition_key() ||
:
std::prev(itr)->second->partition().partition_key_end().empty())
> Likewise, are these std::prev() calls safe? What if itr == end()? Is a prev
std::prev is safe in this case, since we know the collection is non-empty, and
that itr is not pointing to the first element (::begin()).
Line 811: ts_cache_.clear();
> This doesn't leak the map values?
Done
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:
Line 308: else if (!s.ok()) {
: return s;
: }
> Can do RETURN_NOT_OK(s) here instead.
Done
--
To view, visit http://gerrit.cloudera.org:8080/3255
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#4). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 585 insertions(+), 295 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/4 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#3). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 579 insertions(+), 295 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/3 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Adar Dembo has posted comments on this change.
Change subject: KUDU-1308 [c++-client]: support tables with non-covering range
partitions
..
Patch Set 2:
(15 comments)
I reviewed everything but the change to flex_partitioning-itest.cc; will look
at that in the next pass.
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
Line 140: vector required_feature_flags) {
This is never used?
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 383: // Creates a table with 'num_replicas', split into tablets based on
'split_rows'
: // (or single tablet if 'split_rows' is empty).
Update this comment.
Line 1045: KuduPartialRow* a_lower_bound = schema_.NewRow();
Nit: to be completely safe, we ought to store these in unique_ptrs, then
release() them into CreateTable(). You might find this to be too verbose, but I
thought I'd suggest it anyway.
Line 1061: ASSERT_NO_FATAL_FAILURE(InsertTestRows(table.get(), 50, 0));
: client_->data_->meta_cache_->ClearCacheForTesting();
: ASSERT_NO_FATAL_FAILURE(InsertTestRows(table.get(), 50, 50));
: client_->data_->meta_cache_->ClearCacheForTesting();
: ASSERT_NO_FATAL_FAILURE(InsertTestRows(table.get(), 100, 200));
: client_->data_->meta_cache_->ClearCacheForTesting();
What's the significance of clearing the cache? Add a comment?
Line 1068: // Insert an out-of-range rows
Nit: no need for 'an'.
Line 1084: Status result = session->Flush();
If you're going to flush with each row, how about using
KuduSession::AUTO_FLUSH_SYNC?
Line 1140: {
Nit: one-line comments for these test cases too?
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 368: Add a partition range bound to the table with an inclusive lower
bound and
: // exclusive upper bound.
I thought we were going to offer inclusive upper bounds? That's what I remember
from your design doc.
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 755: if (tablets_by_key.empty()) {
Should we do this before upper_bound(), since it doesn't depend on its result?
Line 756: return Status::NotFound("No tablet covering the requested range
partition");
Nit: you're creating the same Status in three different places. Perhaps use a
bool to track the fact, then create it just once? Would be nice to emit the
partition key into the status too, if that's not hard.
Line 758: // The requested partition key is before all tablets
Nit: end with period. Do the same for the others here, please.
Line 762: *remote_tablet = itr->second;
Is the dereference on the RHS safe? Does itr == begin() imply that there's an
element (i.e. does it also imply itr != end())?
Line 763: std::prev(itr)->second->partition().partition_key_end() >
rpc.partition_key() ||
:
std::prev(itr)->second->partition().partition_key_end().empty())
Likewise, are these std::prev() calls safe? What if itr == end()? Is a previous
element guaranteed to exist?
Line 811: ts_cache_.clear();
This doesn't leak the map values?
http://gerrit.cloudera.org:8080/#/c/3255/2/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:
Line 308: else if (!s.ok()) {
: return s;
: }
Can do RETURN_NOT_OK(s) here instead.
--
To view, visit http://gerrit.cloudera.org:8080/3255
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3255 to look at the new patch set (#2). Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 569 insertions(+), 293 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/2 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1308 [c++-client]: support tables with non-covering range partitions
Hello Adar Dembo, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3255 to review the following change. Change subject: KUDU-1308 [c++-client]: support tables with non-covering range partitions .. KUDU-1308 [c++-client]: support tables with non-covering range partitions This commit introduces range bounds to the C++ client's table creation options. Specifying range bounds allows applications to create tables with non-covering range partitions, as described in the non-covering range partitions design document. Additionally, the client is updated to work with tables with non-covering range partitions. Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 --- M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/client/table_creator-internal.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/integration-tests/flex_partitioning-itest.cc 16 files changed, 567 insertions(+), 293 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/3255/1 -- To view, visit http://gerrit.cloudera.org:8080/3255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1cb12704c5e9792ee6e5831568bc52b1a713f8d5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Todd Lipcon
