From 01e40719a465c57bb6beeed497fa15c8ecdcb206 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Fri, 21 Apr 2017 09:36:16 -0700 Subject: [PATCH] Move protobuf conversion code to utils. (#58) * Move protobuf conversation code to utils. * fix format --- mixerclient/BUILD | 2 + mixerclient/include/client.h | 5 --- mixerclient/include/options.h | 49 +++------------------- mixerclient/prefetch/quota_prefetch.cc | 7 ++++ mixerclient/prefetch/quota_prefetch.h | 3 +- mixerclient/src/attribute_context.cc | 23 +---------- mixerclient/src/client_impl.cc | 7 +--- mixerclient/src/client_impl_test.cc | 4 +- mixerclient/utils/protobuf.cc | 56 ++++++++++++++++++++++++++ mixerclient/utils/protobuf.h | 44 ++++++++++++++++++++ 10 files changed, 118 insertions(+), 82 deletions(-) create mode 100644 mixerclient/utils/protobuf.cc create mode 100644 mixerclient/utils/protobuf.h diff --git a/mixerclient/BUILD b/mixerclient/BUILD index c0ee55e36e9..294a2f1bbe1 100644 --- a/mixerclient/BUILD +++ b/mixerclient/BUILD @@ -39,6 +39,8 @@ cc_library( "src/transport.h", "utils/md5.cc", "utils/md5.h", + "utils/protobuf.cc", + "utils/protobuf.h", "utils/status_test_util.h", ], hdrs = [ diff --git a/mixerclient/include/client.h b/mixerclient/include/client.h index fb2df995610..6f9f9d14d7a 100644 --- a/mixerclient/include/client.h +++ b/mixerclient/include/client.h @@ -35,19 +35,14 @@ struct MixerClientOptions { // Constructor with specified option values. MixerClientOptions(const CheckOptions& check_options, - const ReportOptions& report_options, const QuotaOptions& quota_options) : check_options(check_options), - report_options(report_options), quota_options(quota_options), transport(nullptr) {} // Check options. CheckOptions check_options; - // Report options. - ReportOptions report_options; - // Quota options. QuotaOptions quota_options; diff --git a/mixerclient/include/options.h b/mixerclient/include/options.h index f31d881f73f..a44df55cb38 100644 --- a/mixerclient/include/options.h +++ b/mixerclient/include/options.h @@ -63,62 +63,23 @@ struct CheckOptions { std::vector cache_keys; }; -// Options controlling report behavior. -struct ReportOptions { - // Default constructor. - ReportOptions() : num_entries(10000), flush_interval_ms(1000) {} - - // Constructor. - // cache_entries is the maximum number of cache entries that can be kept in - // the cache. Cache is disabled when cache_entries <= 0. - // flush_cache_entry_interval_ms is the maximum milliseconds before - // report requests are flushed to the server. The cache entry is deleted after - // the flush. - ReportOptions(int cache_entries, int flush_cache_entry_interval_ms) - : num_entries(cache_entries), - flush_interval_ms(flush_cache_entry_interval_ms) {} - - // Maximum number of cache entries kept in the cache. - // Set to 0 will disable caching. - const int num_entries; - - // Maximum milliseconds before report requests are flushed to the - // server. The flush is triggered by a timer. - const int flush_interval_ms; -}; - // Options controlling quota behavior. struct QuotaOptions { // Default constructor. - QuotaOptions() - : num_entries(10000), flush_interval_ms(500), expiration_ms(1000) {} + QuotaOptions() : num_entries(10000), expiration_ms(600000) {} // Constructor. // cache_entries is the maximum number of cache entries that can be kept in // the cache. Cache is disabled when cache_entries <= 0. - // flush_cache_entry_interval_ms is the maximum milliseconds before an - // quota request needs to send to remote server again. - // response_expiration_ms is the maximum milliseconds before a cached quota - // response is invalidated. We make sure that it is at least - // flush_cache_entry_interval_ms + 1. - QuotaOptions(int cache_entries, int flush_cache_entry_interval_ms, - int response_expiration_ms) - : num_entries(cache_entries), - flush_interval_ms(flush_cache_entry_interval_ms), - expiration_ms(std::max(flush_cache_entry_interval_ms + 1, - response_expiration_ms)) {} + // expiration_ms is the maximum milliseconds an idle cached quota is removed. + QuotaOptions(int cache_entries, int expiration_ms) + : num_entries(cache_entries), expiration_ms(expiration_ms) {} // Maximum number of cache entries kept in the cache. // Set to 0 will disable caching. const int num_entries; - // Maximum milliseconds before quota requests are flushed to the - // server. The flush is triggered by a quota request. - const int flush_interval_ms; - - // Maximum milliseconds before a cached quota response should be deleted. The - // deletion is triggered by a timer. This value must be larger than - // flush_interval_ms. + // Maximum milliseconds before an idle cached quota should be deleted. const int expiration_ms; }; diff --git a/mixerclient/prefetch/quota_prefetch.cc b/mixerclient/prefetch/quota_prefetch.cc index fb723dfc5f6..862c009f9cc 100644 --- a/mixerclient/prefetch/quota_prefetch.cc +++ b/mixerclient/prefetch/quota_prefetch.cc @@ -244,6 +244,13 @@ void QuotaPrefetchImpl::OnResponse(SlotId slot_id, int req_amount, << ", expire: " << expiration.count() << ", id: " << slot_id << std::endl; + // resp_amount of -1 indicates any network failures. + // Use fail open policy to handle any netowrk failures. + if (resp_amount == -1) { + resp_amount = req_amount; + expiration = milliseconds(kMaxExpirationInMs); + } + Slot* slot = nullptr; if (slot_id != 0) { // The prefetched amount was added to the available queue diff --git a/mixerclient/prefetch/quota_prefetch.h b/mixerclient/prefetch/quota_prefetch.h index 45905271297..669fb0de92f 100644 --- a/mixerclient/prefetch/quota_prefetch.h +++ b/mixerclient/prefetch/quota_prefetch.h @@ -51,12 +51,11 @@ class QuotaPrefetch { // Define the transport. // The callback function after quota allocation is done from the server. + // Set amount = -1 If there are any network failures. typedef std::function DoneFunc; // The transport function to send quota allocation to server. - // Rate limiting usually follows fail-open policy. If there are any network - // failures, the caller should grant the full request amount. typedef std::function TransportFunc; // virtual destructor. diff --git a/mixerclient/src/attribute_context.cc b/mixerclient/src/attribute_context.cc index cfa7312da56..13660506d75 100644 --- a/mixerclient/src/attribute_context.cc +++ b/mixerclient/src/attribute_context.cc @@ -13,35 +13,14 @@ * limitations under the License. */ #include "src/attribute_context.h" -#include "google/protobuf/timestamp.pb.h" +#include "utils/protobuf.h" -using ::google::protobuf::Duration; using ::google::protobuf::Map; -using ::google::protobuf::Timestamp; namespace istio { namespace mixer_client { namespace { -// Convert timestamp from time_point to Timestamp -Timestamp CreateTimestamp(std::chrono::system_clock::time_point tp) { - Timestamp time_stamp; - long long nanos = std::chrono::duration_cast( - tp.time_since_epoch()) - .count(); - - time_stamp.set_seconds(nanos / 1000000000); - time_stamp.set_nanos(nanos % 1000000000); - return time_stamp; -} - -Duration CreateDuration(std::chrono::nanoseconds value) { - Duration duration; - duration.set_seconds(value.count() / 1000000000); - duration.set_nanos(value.count() % 1000000000); - return duration; -} - // Compare two string maps to check // 1) any removed keys: ones in the old, but not in the new. // 2) Get all key/value pairs which are the same in the two maps. diff --git a/mixerclient/src/client_impl.cc b/mixerclient/src/client_impl.cc index 8150deea23a..17f3b070bd4 100644 --- a/mixerclient/src/client_impl.cc +++ b/mixerclient/src/client_impl.cc @@ -13,7 +13,7 @@ * limitations under the License. */ #include "src/client_impl.h" -#include +#include "utils/protobuf.h" using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReportResponse; @@ -23,11 +23,6 @@ using ::google::protobuf::util::error::Code; namespace istio { namespace mixer_client { -namespace { -Status ConvertRpcStatus(const ::google::rpc::Status &status) { - return Status(static_cast(status.code()), status.message()); -} -} // namespace MixerClientImpl::MixerClientImpl(const MixerClientOptions &options) : options_(options) { diff --git a/mixerclient/src/client_impl_test.cc b/mixerclient/src/client_impl_test.cc index 1f7bb484635..05ce1a10357 100644 --- a/mixerclient/src/client_impl_test.cc +++ b/mixerclient/src/client_impl_test.cc @@ -59,9 +59,7 @@ class MixerClientImplTest : public ::testing::Test, public TransportInterface { MixerClientOptions options( CheckOptions(1 /*entries */, 500 /* refresh_interval_ms */, 1000 /* expiration_ms */), - ReportOptions(1 /* entries */, 500 /*flush_interval_ms*/), - QuotaOptions(1 /* entries */, 500 /*flush_interval_ms*/, - 1000 /* expiration_ms */)); + QuotaOptions(1 /* entries */, 600000 /* expiration_ms */)); options.transport = this; client_ = CreateMixerClient(options); } diff --git a/mixerclient/utils/protobuf.cc b/mixerclient/utils/protobuf.cc new file mode 100644 index 00000000000..fd795c0c195 --- /dev/null +++ b/mixerclient/utils/protobuf.cc @@ -0,0 +1,56 @@ +/* Copyright 2017 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "protobuf.h" + +using namespace std::chrono; + +using ::google::protobuf::Duration; +using ::google::protobuf::Timestamp; + +using ::google::protobuf::util::Status; +using ::google::protobuf::util::error::Code; + +namespace istio { +namespace mixer_client { + +// Convert timestamp from time_point to Timestamp +Timestamp CreateTimestamp(system_clock::time_point tp) { + Timestamp time_stamp; + long long nanos = duration_cast(tp.time_since_epoch()).count(); + + time_stamp.set_seconds(nanos / 1000000000); + time_stamp.set_nanos(nanos % 1000000000); + return time_stamp; +} + +Duration CreateDuration(nanoseconds value) { + Duration duration; + duration.set_seconds(value.count() / 1000000000); + duration.set_nanos(value.count() % 1000000000); + return duration; +} + +milliseconds ToMilliseonds(const Duration& duration) { + return duration_cast(seconds(duration.seconds())) + + duration_cast(nanoseconds(duration.nanos())); +} + +Status ConvertRpcStatus(const ::google::rpc::Status& status) { + return Status(static_cast(status.code()), status.message()); +} + +} // namespace mixer_client +} // namespace istio diff --git a/mixerclient/utils/protobuf.h b/mixerclient/utils/protobuf.h new file mode 100644 index 00000000000..191649e259d --- /dev/null +++ b/mixerclient/utils/protobuf.h @@ -0,0 +1,44 @@ +/* Copyright 2017 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef MIXER_CLIENT_UTILS_PROTOBUF_H_ +#define MIXER_CLIENT_UTILS_PROTOBUF_H_ + +#include "google/protobuf/stubs/status.h" +#include "google/protobuf/timestamp.pb.h" +#include "mixer/v1/service.grpc.pb.h" + +namespace istio { +namespace mixer_client { + +// Convert system_clock time to protobuf timestamp +::google::protobuf::Timestamp CreateTimestamp( + std::chrono::system_clock::time_point tp); + +// Convert from chrono duration to protobuf duration. +::google::protobuf::Duration CreateDuration(std::chrono::nanoseconds value); + +// Convert from prtoobuf duration to chrono duration. +std::chrono::milliseconds ToMilliseonds( + const ::google::protobuf::Duration& duration); + +// Convert from grpc status to protobuf status. +::google::protobuf::util::Status ConvertRpcStatus( + const ::google::rpc::Status& status); + +} // namespace mixer_client +} // namespace istio + +#endif // MIXER_CLIENT_CLIENT_UTILS_PROTOBUF_H_