This is an automated email from the ASF dual-hosted git repository.
awong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new c8daf9a [rpc] Add RPC feature flag for Bloom filter predicate
c8daf9a is described below
commit c8daf9a6cdc2fd002b82d3e01a2f58ff714f30d6
Author: Bankim Bhavsar <[email protected]>
AuthorDate: Mon Mar 23 15:26:53 2020 -0700
[rpc] Add RPC feature flag for Bloom filter predicate
If a newer C++ client communicates with an older tablet server
that doesn't implement Bloom filter predicate push-down
then the tablet server will simply drop the protobuf message.
This change adds a RPC feature that'll report unsupported
feature flag error if a newer client were to send
Bloom filter predicate to an older tablet server.
Thought about 2 options:
1) Add a new state using a counter in ScanSpec class that tracks
whether any of the "predicates_" is a Bloom filter predicate.
This state will need to be managed for addition, removal, merging of
predicates etc.
Pros: Efficient since it'll be a single check when sending RPC.
Cons: Need to maintain additional state.
2) Don't add new state and use the existing "predicates_"
map in ScanSpec class to determine whether any Bloom filter
predicates are being sent.
Pros: Doesn't require tracking any additional state.
Cons: Can be inefficient with large number of predicates.
With both options, check for presence of Bloom filter predicates can be
skipped for scan continuations reducing perf impact on the client.
Opted for option#2, since number of predicates on a table
are expected to be small(unless the table has large number of columns)
and doesn't require maintaining additional state.
Tests:
- In tablet_service.cc SupportsFeature() function commented out
BLOOM_FILTER_PREDICATE and verified unit tests failed
with unsupported feature flag 4 error log message.
Change-Id: I87c6bac1e9f77a9ea306ab00404e5101a0c583b9
Reviewed-on: http://gerrit.cloudera.org:8080/15540
Reviewed-by: Adar Dembo <[email protected]>
Tested-by: Kudu Jenkins
---
src/kudu/client/scanner-internal.cc | 12 ++++++++++++
src/kudu/common/scan_spec.cc | 7 +++++++
src/kudu/common/scan_spec.h | 3 +++
src/kudu/tserver/tablet_service.cc | 2 ++
src/kudu/tserver/tserver.proto | 1 +
5 files changed, 25 insertions(+)
diff --git a/src/kudu/client/scanner-internal.cc
b/src/kudu/client/scanner-internal.cc
index f552d4e..9cb14aa 100644
--- a/src/kudu/client/scanner-internal.cc
+++ b/src/kudu/client/scanner-internal.cc
@@ -22,6 +22,7 @@
#include <ostream>
#include <string>
#include <unordered_map>
+#include <unordered_set>
#include <utility>
#include <vector>
@@ -38,6 +39,7 @@
#include "kudu/common/scan_spec.h"
#include "kudu/common/schema.h"
#include "kudu/common/wire_protocol.h"
+#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/strings/stringpiece.h"
#include "kudu/gutil/strings/substitute.h"
@@ -347,10 +349,20 @@ ScanRpcStatus KuduScanner::Data::SendScanRpc(const
MonoTime& overall_deadline,
rpc_deadline = overall_deadline;
}
+ // Capture previously sent Bloom filter predicate feature flag so that we
don't have to make
+ // expensive call to determine the flag on scan continuations.
+ bool prev_bloom_filter_feature =
ContainsKey(controller_.required_server_features(),
+
TabletServerFeatures::BLOOM_FILTER_PREDICATE);
+
controller_.Reset();
controller_.set_deadline(rpc_deadline);
if (!configuration_.spec().predicates().empty()) {
controller_.RequireServerFeature(TabletServerFeatures::COLUMN_PREDICATES);
+ if (prev_bloom_filter_feature ||
+ (next_req_.has_new_scan_request() &&
+ configuration().spec().ContainsBloomFilterPredicate())) {
+
controller_.RequireServerFeature(TabletServerFeatures::BLOOM_FILTER_PREDICATE);
+ }
}
if (configuration().row_format_flags() &
KuduScanner::PAD_UNIXTIME_MICROS_TO_16_BYTES) {
controller_.RequireServerFeature(TabletServerFeatures::PAD_UNIXTIME_MICROS_TO_16_BYTES);
diff --git a/src/kudu/common/scan_spec.cc b/src/kudu/common/scan_spec.cc
index b336409..fd93567 100644
--- a/src/kudu/common/scan_spec.cc
+++ b/src/kudu/common/scan_spec.cc
@@ -87,6 +87,13 @@ bool ScanSpec::CanShortCircuit() const {
});
}
+bool ScanSpec::ContainsBloomFilterPredicate() const {
+ return any_of(predicates_.begin(), predicates_.end(),
+ [] (const pair<string, ColumnPredicate>& col_pred_pair) {
+ return col_pred_pair.second.predicate_type() ==
PredicateType::InBloomFilter;
+ });
+}
+
void ScanSpec::SetLowerBoundKey(const EncodedKey* key) {
if (lower_bound_key_ == nullptr ||
key->encoded_key().compare(lower_bound_key_->encoded_key()) > 0) {
diff --git a/src/kudu/common/scan_spec.h b/src/kudu/common/scan_spec.h
index 224694f..49fde73 100644
--- a/src/kudu/common/scan_spec.h
+++ b/src/kudu/common/scan_spec.h
@@ -58,6 +58,9 @@ class ScanSpec {
// Returns true if the result set is known to be empty.
bool CanShortCircuit() const;
+ // Returns true if a Bloom filter predicate is present.
+ bool ContainsBloomFilterPredicate() const;
+
// Optimizes the scan by unifying the lower and upper bound constraints and
// the column predicates.
//
diff --git a/src/kudu/tserver/tablet_service.cc
b/src/kudu/tserver/tablet_service.cc
index b120661..e3f861b 100644
--- a/src/kudu/tserver/tablet_service.cc
+++ b/src/kudu/tserver/tablet_service.cc
@@ -1017,6 +1017,7 @@ bool TabletServiceAdminImpl::SupportsFeature(uint32_t
feature) const {
case TabletServerFeatures::COLUMN_PREDICATES:
case TabletServerFeatures::PAD_UNIXTIME_MICROS_TO_16_BYTES:
case TabletServerFeatures::QUIESCING:
+ case TabletServerFeatures::BLOOM_FILTER_PREDICATE:
return true;
default:
return false;
@@ -2109,6 +2110,7 @@ bool TabletServiceImpl::SupportsFeature(uint32_t feature)
const {
case TabletServerFeatures::COLUMN_PREDICATES:
case TabletServerFeatures::PAD_UNIXTIME_MICROS_TO_16_BYTES:
case TabletServerFeatures::QUIESCING:
+ case TabletServerFeatures::BLOOM_FILTER_PREDICATE:
return true;
default:
return false;
diff --git a/src/kudu/tserver/tserver.proto b/src/kudu/tserver/tserver.proto
index 3c5599a..f2ab6d4 100644
--- a/src/kudu/tserver/tserver.proto
+++ b/src/kudu/tserver/tserver.proto
@@ -471,4 +471,5 @@ enum TabletServerFeatures {
// Whether the server supports padding UNIXTIME_MICROS slots to 16 bytes.
PAD_UNIXTIME_MICROS_TO_16_BYTES = 2;
QUIESCING = 3;
+ BLOOM_FILTER_PREDICATE = 4;
}
