aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/grpc/impl/codegen/grpc_types.h8
-rw-r--r--src/core/lib/channel/channel_trace.cc10
-rw-r--r--src/core/lib/channel/channel_trace.h20
-rw-r--r--src/core/lib/channel/channelz.h3
-rw-r--r--test/core/channel/channel_trace_test.cc23
-rw-r--r--test/core/end2end/tests/channelz.cc18
6 files changed, 45 insertions, 37 deletions
diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h
index 42daf3bb6e..3ce88a8264 100644
--- a/include/grpc/impl/codegen/grpc_types.h
+++ b/include/grpc/impl/codegen/grpc_types.h
@@ -285,10 +285,10 @@ typedef struct {
#define GRPC_ARG_SOCKET_MUTATOR "grpc.socket_mutator"
/** The grpc_socket_factory instance to create and bind sockets. A pointer. */
#define GRPC_ARG_SOCKET_FACTORY "grpc.socket_factory"
-/** The maximum ammount of memory that the trace event can use per channel
- * trace node. Once the maximum is reached, subsequent events will evict the
- * oldest events from the buffer. The unit for this knob is bytes. Setting
- * it to zero causes channel tracing to be disabled. */
+/** The maximum amount of memory used by trace events per channel trace node.
+ * Once the maximum is reached, subsequent events will evict the oldest events
+ * from the buffer. The unit for this knob is bytes. Setting it to zero causes
+ * channel tracing to be disabled. */
#define GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE \
"grpc.max_channel_trace_event_memory_per_node"
/** If non-zero, gRPC library will track stats and information at at per channel
diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc
index 3227fe53a1..fe81acb617 100644
--- a/src/core/lib/channel/channel_trace.cc
+++ b/src/core/lib/channel/channel_trace.cc
@@ -48,18 +48,16 @@ ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data,
timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(),
GPR_CLOCK_REALTIME)),
next_(nullptr),
- referenced_entity_(std::move(referenced_entity)) {
- memory_usage_ = sizeof(TraceEvent) + grpc_slice_memory_usage(data);
-}
+ referenced_entity_(std::move(referenced_entity)),
+ memory_usage_(sizeof(TraceEvent) + grpc_slice_memory_usage(data)) {}
ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data)
: severity_(severity),
data_(data),
timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(),
GPR_CLOCK_REALTIME)),
- next_(nullptr) {
- memory_usage_ = sizeof(TraceEvent) + grpc_slice_memory_usage(data);
-}
+ next_(nullptr),
+ memory_usage_(sizeof(TraceEvent) + grpc_slice_memory_usage(data)) {}
ChannelTrace::TraceEvent::~TraceEvent() { grpc_slice_unref_internal(data_); }
diff --git a/src/core/lib/channel/channel_trace.h b/src/core/lib/channel/channel_trace.h
index 0e9ce5f648..8ff91ee8c8 100644
--- a/src/core/lib/channel/channel_trace.h
+++ b/src/core/lib/channel/channel_trace.h
@@ -30,6 +30,10 @@
namespace grpc_core {
namespace channelz {
+namespace testing {
+size_t GetSizeofTraceEvent(void);
+}
+
class BaseNode;
// Object used to hold live data for a channel. This data is exposed via the
@@ -49,6 +53,12 @@ class ChannelTrace {
// Adds a new trace event to the tracing object
//
+ // NOTE: each ChannelTrace tracks the memory used by its list of trace
+ // events, so adding an event with a large amount of data could cause other
+ // trace event to be evicted. If a single trace is larger than the limit, it
+ // will cause all events to be evicted. The limit is set with the arg:
+ // GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE.
+ //
// TODO(ncteisen): as this call is used more and more throughout the gRPC
// stack, determine if it makes more sense to accept a char* instead of a
// slice.
@@ -59,9 +69,9 @@ class ChannelTrace {
// channel has created a new subchannel, then it would record that with
// a TraceEvent referencing the new subchannel.
//
- // TODO(ncteisen): as this call is used more and more throughout the gRPC
- // stack, determine if it makes more sense to accept a char* instead of a
- // slice.
+ // NOTE: see the note in the method above.
+ //
+ // TODO(ncteisen): see the todo in the method above.
void AddTraceEventWithReference(Severity severity, grpc_slice data,
RefCountedPtr<BaseNode> referenced_entity);
@@ -70,6 +80,8 @@ class ChannelTrace {
grpc_json* RenderJson() const;
private:
+ friend size_t testing::GetSizeofTraceEvent(void);
+
// Private class to encapsulate all the data and bookkeeping needed for a
// a trace event.
class TraceEvent {
@@ -92,7 +104,7 @@ class ChannelTrace {
TraceEvent* next() const { return next_; }
void set_next(TraceEvent* next) { next_ = next; }
- size_t memory_usage() { return memory_usage_; }
+ size_t memory_usage() const { return memory_usage_; }
private:
Severity severity_;
diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h
index 97eef9868d..30ec66ced2 100644
--- a/src/core/lib/channel/channelz.h
+++ b/src/core/lib/channel/channelz.h
@@ -43,7 +43,8 @@
* GRPC_ARG_ENABLE_CHANNELZ is set, it will override this default value. */
#define GRPC_ENABLE_CHANNELZ_DEFAULT false
-/** This is the default value for TODO. If
+/** This is the default value for the maximum amount of memory used by trace
+ * events per channel trace node. If
* GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE is set, it will override
* this default value. */
#define GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT 0
diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc
index 8385475e7d..a569444fed 100644
--- a/test/core/channel/channel_trace_test.cc
+++ b/test/core/channel/channel_trace_test.cc
@@ -30,6 +30,7 @@
#include "src/core/lib/gpr/useful.h"
#include "src/core/lib/iomgr/exec_ctx.h"
#include "src/core/lib/json/json.h"
+#include "src/core/lib/surface/channel.h"
#include "test/core/util/test_config.h"
#include "test/cpp/util/channel_trace_proto_helper.h"
@@ -51,6 +52,8 @@ class ChannelNodePeer {
ChannelNode* node_;
};
+size_t GetSizeofTraceEvent() { return sizeof(ChannelTrace::TraceEvent); }
+
namespace {
grpc_json* GetJsonChild(grpc_json* parent, const char* key) {
@@ -119,11 +122,9 @@ void ValidateChannelTrace(ChannelTrace* tracer, size_t num_events_logged) {
class ChannelFixture {
public:
ChannelFixture(int max_tracer_event_memory) {
- grpc_arg client_a;
- client_a.type = GRPC_ARG_INTEGER;
- client_a.key =
- const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE);
- client_a.value.integer = max_tracer_event_memory;
+ grpc_arg client_a = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE),
+ max_tracer_event_memory);
grpc_channel_args client_args = {1, &client_a};
channel_ =
grpc_insecure_channel_create("fake_target", &client_args, nullptr);
@@ -286,9 +287,9 @@ TEST(ChannelTracerTest, TestSmallMemoryLimit) {
TEST(ChannelTracerTest, TestEviction) {
grpc_core::ExecCtx exec_ctx;
- // This depends on sizeof(ChannelTrace). If that struct has been updated,
+ // This depends on sizeof(ChannelEvent). If that struct has been updated,
// this will too.
- const int kTraceEventSize = 80;
+ const int kTraceEventSize = GetSizeofTraceEvent();
const int kNumEvents = 5;
ChannelTrace tracer(kTraceEventSize * kNumEvents);
for (int i = 1; i <= kNumEvents; ++i) {
@@ -305,9 +306,9 @@ TEST(ChannelTracerTest, TestEviction) {
TEST(ChannelTracerTest, TestMultipleEviction) {
grpc_core::ExecCtx exec_ctx;
- // This depends on sizeof(ChannelTrace). If that struct has been updated,
+ // This depends on sizeof(ChannelEvent). If that struct has been updated,
// this will too.
- const int kTraceEventSize = 80;
+ const int kTraceEventSize = GetSizeofTraceEvent();
const int kNumEvents = 5;
ChannelTrace tracer(kTraceEventSize * kNumEvents);
for (int i = 1; i <= kNumEvents; ++i) {
@@ -326,9 +327,9 @@ TEST(ChannelTracerTest, TestMultipleEviction) {
TEST(ChannelTracerTest, TestTotalEviction) {
grpc_core::ExecCtx exec_ctx;
- // This depends on sizeof(ChannelTrace). If that struct has been updated,
+ // This depends on sizeof(ChannelEvent). If that struct has been updated,
// this will too.
- const int kTraceEventSize = 80;
+ const int kTraceEventSize = GetSizeofTraceEvent();
const int kNumEvents = 5;
ChannelTrace tracer(kTraceEventSize * kNumEvents);
for (int i = 1; i <= kNumEvents; ++i) {
diff --git a/test/core/end2end/tests/channelz.cc b/test/core/end2end/tests/channelz.cc
index 58ffdc18da..0e3ad6a476 100644
--- a/test/core/end2end/tests/channelz.cc
+++ b/test/core/end2end/tests/channelz.cc
@@ -199,10 +199,8 @@ static void run_one_request(grpc_end2end_test_config config,
static void test_channelz(grpc_end2end_test_config config) {
grpc_end2end_test_fixture f;
- grpc_arg arg;
- arg.type = GRPC_ARG_INTEGER;
- arg.key = const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ);
- arg.value.integer = true;
+ grpc_arg arg = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true);
grpc_channel_args args = {1, &arg};
f = begin_test(config, "test_channelz", &args, &args);
@@ -266,13 +264,11 @@ static void test_channelz_with_channel_trace(grpc_end2end_test_config config) {
grpc_end2end_test_fixture f;
grpc_arg arg[2];
- arg[0].type = GRPC_ARG_INTEGER;
- arg[0].key =
- const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE);
- arg[0].value.integer = 1024;
- arg[1].type = GRPC_ARG_INTEGER;
- arg[1].key = const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ);
- arg[1].value.integer = true;
+ arg[0] = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE),
+ 1024 * 1024);
+ arg[1] = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true);
grpc_channel_args args = {GPR_ARRAY_SIZE(arg), arg};
f = begin_test(config, "test_channelz_with_channel_trace", &args, &args);
generated by cgit v1.2.3 (git 2.39.1) at 2024-06-11 08:16:24 +0000