8000 fix traffic accounting in internal statistics / metrics (#17654) · arangodb/arangodb@ca7ac5e · GitHub
[go: up one dir, main page]
Skip to content

Commit ca7ac5e

Browse files
jsteemannKVS85
jsteemann
and
KVS85
authored
fix traffic accounting in internal statistics / metrics (#17654)
* fix traffic accounting in internal statistics / metrics * fix HTTP2 + SSL in test env * remove backport leftover * add vst+ssl test Co-authored-by: Vadim <vadim@arangodb.com>
1 parent 24ba39d commit ca7ac5e

16 files changed

+360
-161
lines changed

‎CHANGELOG

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
v3.8.9 (XXXX-XX-XX)
22
-------------------
33

4+
* Fix HTTP/VST traffic accounting in internal statistics / metrics.
5+
46
* Delay a MoveShard operation for leader change, until the old leader has
57
actually assumed its leadership and until the new leader is actually in sync.
68
This fixes a bug which could block a shard under certain circumstances. This

‎Documentation/Metrics/arangodb_http1_connections_total.yaml

Lines changed: 16 additions & 0 deletions
A36C
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
name: arangodb_http1_connections_total
2+
introducedIn: "3.9.6"
3+
help: |
4+
Total number of HTTP/1.1 connections accepted.
5+
unit: number
6+
type: counter
7+
category: Connectivity
8+
complexity: medium
9+
exposedBy:
10+
- coordinator
11+
- dbserver
12+
- agent
13+
- single
14+
description: |
15+
Total number of connections accepted for HTTP/1.1. Note that this can
16+
include connections that are negotiated to be upgraded to HTTP/2.

‎arangod/GeneralServer/CommTask.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,11 @@ void CommTask::executeRequest(std::unique_ptr<GeneralRequest> request,
420420
// --SECTION-- statistics handling protected methods
421421
// -----------------------------------------------------------------------------
422422

423+
void CommTask::setStatistics(uint64_t id, RequestStatistics::Item&& stat) {
424+
std::lock_guard<std::mutex> guard(_statisticsMutex);
425+
_statisticsMap.insert_or_assign(id, std::move(stat));
426+
}
427+
423428
RequestStatistics::Item const& CommTask::acquireStatistics(uint64_t id) {
424429
RequestStatistics::Item stat = RequestStatistics::acquire();
425430

‎arangod/GeneralServer/CommTask.h

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,13 @@
3030
#include "Statistics/ConnectionStatistics.h"
3131
#include "Statistics/RequestStatistics.h"
3232

33+
#include <velocypack/Buffer.h>
34+
35+
#include <chrono>
36+
#include <cstdint>
37+
#include <memory>
3338
#include <mutex>
39+
#include <unordered_map>
3440

3541
namespace arangodb {
3642
class AuthenticationFeature;
@@ -90,6 +96,8 @@ class CommTask : public std::enable_shared_from_this<CommTask> {
9096
virtual void start() = 0;
9197
virtual void stop() = 0;
9298

99+
void setStatistics(uint64_t id, RequestStatistics::Item&& stat);
100+
93101
protected:
94102

95103
virtual std::unique_ptr<GeneralResponse> createResponse(rest::ResponseCode,
@@ -115,10 +123,10 @@ class CommTask : public std::enable_shared_from_this<CommTask> {
115123
void executeRequest(std::unique_ptr<GeneralRequest>,
116124
std::unique_ptr<GeneralResponse>);
117125

118-
RequestStatistics::Item const& acquireStatistics(uint64_t);
119-
RequestStatistics::Item const& statistics(uint64_t);
120-
RequestStatistics::Item stealStatistics(uint64_t);
121-
126+
RequestStatistics::Item const& acquireStatistics(uint64_t id);
127+
RequestStatistics::Item const& statistics(uint64_t id);
128+
RequestStatistics::Item stealStatistics(uint64_t id);
129+
122130
/// @brief send response including error response body
123131
void sendErrorResponse(rest::ResponseCode, rest::ContentType,
124132
uint64_t messageId, ErrorCode errorNum,
@@ -150,18 +158,17 @@ class CommTask : public std::enable_shared_from_this<CommTask> {
150158
private:
151159
bool handleRequestSync(std::shared_ptr<RestHandler>);
152160
bool handleRequestAsync(std::shared_ptr<RestHandler>, uint64_t* jobId = nullptr);
153-
161+
162+
mutable std::mutex _statisticsMutex;
163+
std::unordered_map<uint64_t, RequestStatistics::Item> _statisticsMap;
164+
154165
protected:
155-
156166
GeneralServer& _server;
157167
ConnectionInfo _connectionInfo;
158168

159169
ConnectionStatistics::Item _connectionStatistics;
160170
std::chrono::milliseconds _keepAliveTimeout;
161171
AuthenticationFeature* _auth;
162-
163-
mutable std::mutex _statisticsMutex;
164-
std::unordered_map<uint64_t, RequestStatistics::Item> _statisticsMap;
165172
};
166173
} // namespace rest
167174
} // namespace arangodb

‎arangod/GeneralServer/GeneralServerFeature.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ DECLARE_HISTOGRAM(arangodb_request_body_size_http2, RequestBodySizeScale,
131131
"Body size of HTTP/2 requests");
132132
DECLARE_HISTOGRAM(arangodb_request_body_size_vst, RequestBodySizeScale,
133133
"Body size of VST requests");
134+
DECLARE_COUNTER(arangodb_http1_connections_total,
135+
"Total number of HTTP/1.1 connections");
134136
DECLARE_COUNTER(arangodb_http2_connections_total,
135137
"Total number of HTTP/2 connections");
136138
DECLARE_COUNTER(arangodb_vst_connections_total,
@@ -149,6 +151,8 @@ GeneralServerFeature::GeneralServerFeature(application_features::ApplicationServ
149151
server.getFeature<MetricsFeature>().add(arangodb_request_body_size_http2{})),
150152
_requestBodySizeVst(
151153
server.getFeature<MetricsFeature>().add(arangodb_request_body_size_vst{})),
154+
_http1Connections(
155+
server.getFeature<MetricsFeature>().add(arangodb_http1_connections_total{})),
152156
_http2Connections(
153157
server.getFeature<MetricsFeature>().add(arangodb_http2_connections_total{})),
154158
_vstConnections(

‎arangod/GeneralServer/GeneralServerFeature.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ class GeneralServerFeature final : public application_features::ApplicationFeatu
7070
_requestBodySizeVst.count(bodySize);
7171
}
7272

73-
void countHttp2Connection() {
74-
_http2Connections.count();
75-
}
73+
void countHttp1Connection() { _http1Connections.count(); }
74+
75+
void countHttp2Connection() { _http2Connections.count(); }
7676

7777
void countVstConnection() {
7878
_vstConnections.count();
@@ -100,6 +100,7 @@ class GeneralServerFeature final : public application_features::ApplicationFeatu
100100
Histogram<log_scale_t<uint64_t>>& _requestBodySizeHttp1;
101101
Histogram<log_scale_t<uint64_t>>& _requestBodySizeHttp2;
102102
Histogram<log_scale_t<uint64_t>>& _requestBodySizeVst;
103+
Counter& _http1Connections;
103104
Counter& _http2Connections;
104105
Counter& _vstConnections;
105106
};

‎arangod/GeneralServer/H2CommTask.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ template <SocketType T>
6262
return 0;
6363
}
6464

65-
const int32_t sid = frame->hd.stream_id;
65+
int32_t const sid = frame->hd.stream_id;
6666
me->acquireStatistics(sid).SET_READ_START(TRI_microtime());
6767
auto req = std::make_unique<HttpRequest>(me->_connectionInfo, /*messageId*/ sid,
6868
/*allowMethodOverride*/ false);
@@ -79,7 +79,7 @@ template <SocketType T>
7979
const uint8_t* value, size_t valuelen,
8080
uint8_t flags, void* user_data) {
8181
H2CommTask<T>* me = static_cast<H2CommTask<T>*>(user_data);
82-
const int32_t sid = frame->hd.stream_id;
82+
int32_t const sid = frame->hd.stream_id;
8383

8484
if (frame->hd.type != NGHTTP2_HEADERS || frame->headers.cat != NGHTTP2_HCAT_REQUEST) {
8585
return 0;
@@ -125,7 +125,7 @@ template <SocketType T>
125125
case NGHTTP2_DATA: // GET or HEAD do not use DATA frames
126126
case NGHTTP2_HEADERS: {
127127
if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
128-
const int32_t sid = frame->hd.stream_id;
128+
int32_t const sid = frame->hd.stream_id;
129129
LOG_TOPIC("c75b1", TRACE, Logger::REQUESTS)
130130
<< "<http2> finalized request on stream " << sid << " with ptr " << me;
131131

@@ -193,7 +193,7 @@ template <SocketType T>
193193
return 0;
194194
}
195195

196-
const int32_t sid = frame->hd.stream_id;
196+
int32_t const sid = frame->hd.stream_id;
197197
LOG_TOPIC("d15e8", DEBUG, Logger::REQUESTS)
198198
<< "sending RST on stream " << sid << " with error '"
199199
<< nghttp2_strerror(lib_error_code) << "' (" << lib_error_code << ")";
@@ -708,7 +708,7 @@ void H2CommTask<T>::queueHttp2Responses() {
708708
size_t nread = std::min(length, body.size() - strm->responseOffset);
709709
TRI_ASSERT(nread > 0);
710710

711-
const char* src = body.data() + strm->responseOffset;
711+
char const* src = body.data() + strm->responseOffset;
712712
std::copy_n(src, nread, buf);
713713
strm->responseOffset += nread;
714714

‎arangod/GeneralServer/HttpCommTask.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ HttpCommTask<T>::HttpCommTask(GeneralServer& server, ConnectionInfo info,
252252
_parserSettings.on_message_complete = HttpCommTask<T>::on_message_complete;
253253
llhttp_init(&_parser, HTTP_REQUEST, &_parserSettings);
254254
_parser.data = this;
255+
256+
this->_generalServerFeature.countHttp1Connection();
255257
}
256258

257259
template <SocketType T>
@@ -274,8 +276,8 @@ bool HttpCommTask<T>::readCallback(asio_ns::error_code ec) {
274276
// Inspect the received data
275277
size_t nparsed = 0;
276278
for (auto const& buffer : this->_protocol->buffer.data()) {
277-
const char* data = reinterpret_cast<const char*>(buffer.data());
278-
const char* end = data + buffer.size();
279+
char const* data = reinterpret_cast<char const*>(buffer.data());
280+
char const* end = data + buffer.size();
279281
do {
280282
size_t datasize = end - data;
281283

@@ -391,6 +393,7 @@ void HttpCommTask<T>::checkVSTPrefix() {
391393
auto commTask = std::make_unique<VstCommTask<T>>(me._server, me._connectionInfo,
392394
std::move(me._protocol),
393395
fuerte::vst::VST1_0);
396+
commTask->setStatistics(1UL, me.stealStatistics(1UL));
394397
me._server.registerTask(std::move(commTask));
395398
me.close(ec);
396399
return; // vst 1.0
@@ -400,6 +403,7 @@ void HttpCommTask<T>::checkVSTPrefix() {
400403
auto commTask = std::make_unique<VstCommTask<T>>(me._server, me._connectionInfo,
401404
std::move(me._protocol),
402405
fuerte::vst::VST1_1);
406+
commTask->setStatistics(1UL, me.stealStatistics(1UL));
403407
me._server.registerTask(std::move(commTask));
404408
me.close(ec);
405409
return; // vst 1.1
@@ -408,6 +412,7 @@ void HttpCommTask<T>::checkVSTPrefix() {
408412
// do not remove preface here, H2CommTask will read it from buffer
409413
auto commTask = std::make_unique<H2CommTask<T>>(me._server, me._connectionInfo,
410414
std::move(me._protocol));
415+
commTask->setStatistics(1UL, me.stealStatistics(1UL));
411416
me._server.registerTask(std::move(commTask));
412417
me.close(ec);
413418
return; // http2 upgrade
@@ -458,6 +463,7 @@ void HttpCommTask<T>::processRequest() {
458463
if (h2 == "h2c" && found && !settings.empty()) {
459464
auto task = std::make_shared<H2CommTask<T>>(this->_server, this->_connectionInfo,
460465
std::move(this->_protocol));
466+
task->setStatistics(1UL, this->stealStatistics(1UL));
461467
task->upgradeHttp1(std::move(_request));
462468
this->close();
463469
return;
@@ -697,7 +703,6 @@ void HttpCommTask<T>::writeResponse(RequestStatistics::Item stat) {
697703
}
698704

699705
this->_writing = true;
700-
// FIXME measure performance w/o sync write
701706
asio_ns::async_write(this->_protocol->socket, buffers,
702707
[self = this->shared_from_this(),
703708
stat = std::move(stat)](asio_ns::error_code ec, size_t nwrite) {

‎arangod/GeneralServer/VstCommTask.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -455,15 +455,14 @@ void VstCommTask<T>::doWrite() {
455455
auto& buffers = item->buffers;
456456
asio_ns::async_write(this->_protocol->socket, buffers,
457457
[self(CommTask::shared_from_this()),
458-
rsp(std::move(item))](asio_ns::error_code const& ec, size_t) {
458+
rsp(std::move(item))](asio_ns::error_code const& ec, size_t nwrite) {
459459
DTraceVstCommTaskAfterAsyncWrite((size_t)self.get());
460460

461461
auto& me = static_cast<VstCommTask<T>&>(*self);
462462
me._writing = false;
463463

464464
rsp->stat.SET_WRITE_END();
465-
rsp->stat.ADD_SENT_BYTES(rsp->buffers[0].size() +
466-
rsp->buffers[1].size());
465+
rsp->stat.ADD_SENT_BYTES(nwrite);
467466
if (ec) {
468467
me.close(ec);
469468
} else {

‎arangod/Statistics/StatisticsFeature.cpp

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ VPackBuilder StatisticsFeature::fillDistribution(statistics::Distribution const&
688688
void StatisticsFeature::appendHistogram(
689689
std::string& result, statistics::Distribution const& dist,
690690
std::string const& label, std::initializer_list<std::string> const& les,
691-
bool v2) {
691+
bool v2, bool isInteger) {
692692
using StringUtils::concatT;
693693

694694
VPackBuilder tmp = fillDistribution(dist);
@@ -713,8 +713,13 @@ void StatisticsFeature::appendHistogram(
713713
}
714714
result += concatT(name, "_count ", sum, "\n");
715715
if (v2) {
716-
double v = slc.get("sum").getNumber<double>();
717-
result += concatT(name, "_sum ", v, "\n");
716+
if (isInteger) {
717+
uint64_t v = slc.get("sum").getNumber<uint64_t>();
718+
result += concatT(name, "_sum ", v, "\n");
719+
} else {
720+
double v = slc.get("sum").getNumber<double>();
721+
result += concatT(name, "_sum ", v, "\n");
722+
}
718723
}
719724
}
720725

@@ -784,21 +789,21 @@ void StatisticsFeature::toPrometheus(std::string& result, double const& now, boo
784789

785790
// _clientStatistics()
786791
appendMetric(result, std::to_string(connectionStats.httpConnections.get()), "clientHttpConnections", v2);
787-
appendHistogram(result, connectionStats.connectionTime, "connectionTime", {"0.01", "1.0", "60.0", "+Inf"}, v2);
788-
appendHistogram(result, requestStats.totalTime, "totalTime",
789-
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", "15.0",
790-
"30.0", "+Inf"}, v2);
791-
appendHistogram(result, requestStats.requestTime, "requestTime",
792-
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", "15.0",
793-
"30.0", "+Inf"}, v2);
794-
appendHistogram(result, requestStats.queueTime, "queueTime",
795-
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", "15.0",
796-
"30.0", "+Inf"}, v2);
797-
appendHistogram(result, requestStats.ioTime, "ioTime",
798-
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", "15.0",
799-
"30.0", "+Inf"}, v2);
800-
appendHistogram(result, requestStats.bytesSent, "bytesSent", {"250", "1000", "2000", "5000", "10000", "+Inf"}, v2);
801-
appendHistogram(result, requestStats.bytesReceived, "bytesReceived", {"250", "1000", "2000", "5000", "10000", "+Inf"}, v2);
792+
appendHistogram(result, connectionStats.connectionTime, "connectionTime", {"0.01", "1.0", "60.0", "+Inf"}, v2, false);
793+
appendHistogram(result, requestStats.totalTime, "totalTime",
794+
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", "15.0",
795+
"30.0", "+Inf"}, v2, false);
796+
appendHistogram(result, requestStats.requestTime, "requestTime",
797+
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", "15.0",
798+
"30.0", "+Inf"}, v2, false);
799+
appendHistogram(result, requestStats.queueTime, "queueTime",
800+
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", "15.0",
801+
"30.0", "+Inf"}, v2, false);
802+
appendHistogram(result, requestStats.ioTime, "ioTime",
803+
{"0.01", "0.05", "0.1", "0.2", "0.5", "1.0", "5.0", " DF89 15.0",
804+
"30.0", "+Inf"}, v2, false);
805+
appendHistogram(result, requestStats.bytesSent, "bytesSent", {"250", "1000", "2000", "5000", "10000", "+Inf"}, v2, true);
806+
appendHistogram(result, requestStats.bytesReceived, "bytesReceived", {"250", "1000", "2000", "5000", "10000", "+Inf"}, v2, true);
802807

803808
// _httpStatistics()
804809
using rest::RequestType;

‎arangod/Statistics/StatisticsFeature.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class StatisticsFeature final : public application_features::ApplicationFeature
107107
static void appendHistogram(
108108
std::string& result, statistics::Distribution const& dist,
109109
std::string const& label, std::initializer_list<std::string> const& les,
110-
bool v2);
110+
bool v2, bool isInteger);
111111
static void appendMetric(
112112
std::string& result, std::string const& val, std::string const& label,
113113
bool v2);
@@ -116,7 +116,7 @@ class StatisticsFeature final : public application_features::ApplicationFeature
116116
double start,
117117
arangodb::velocypack::Builder& result) const;
118118

119-
bool allDatabases() const { return _statisticsAllDatabases; }
119+
bool allDatabases() const noexcept { return _statisticsAllDatabases; }
120120

121121
private:
122122
bool _statistics;

0 commit comments

Comments
 (0)
0