This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new aa319a4 Fix H2 internal counters
aa319a4 is described below
commit aa319a461c8326724ea10a42ae474fcdac7fc849
Author: Masakazu Kitajo <mas...@apache.org>
AuthorDate: Sat Aug 17 00:29:30 2019 +0900
Fix H2 internal counters
---
.gitignore | 1 +
proxy/http2/Http2ConnectionState.cc | 56 +-----
proxy/http2/Http2ConnectionState.h | 17 +-
proxy/http2/Http2FrequencyCounter.cc | 55 ++++++
proxy/http2/Http2FrequencyCounter.h | 38 ++++
proxy/http2/Makefile.am | 17 ++
.../http2/unit_tests/test_Http2FrequencyCounter.cc | 219 +++++++++++++++++++++
7 files changed, 343 insertions(+), 60 deletions(-)
diff --git a/.gitignore b/.gitignore
index 6ac08e2..c683ae3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -130,6 +130,7 @@ proxy/hdrs/test_Huffmancode
proxy/hdrs/test_XPACK
proxy/http/test_proxy_http
proxy/http2/test_Http2DependencyTree
+proxy/http2/test_Http2FrequencyCounter
proxy/http2/test_HPACK
proxy/http2/hpack-tests/results
proxy/http3/test_libhttp3
diff --git a/proxy/http2/Http2ConnectionState.cc
b/proxy/http2/Http2ConnectionState.cc
index fecb360..b26e61e 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1901,89 +1901,49 @@
Http2ConnectionState::send_window_update_frame(Http2StreamId id, uint32_t size)
void
Http2ConnectionState::increment_received_settings_count(uint32_t count)
{
- ink_hrtime hrtime_sec = Thread::get_hrtime() / HRTIME_SECOND;
- uint8_t counter_index = ((hrtime_sec % 60) >= 30);
-
- if ((hrtime_sec - 60) > this->settings_count_last_update) {
- this->settings_count[0] = 0;
- this->settings_count[1] = 0;
- } else if (counter_index != ((this->settings_count_last_update % 60) >= 30))
{
- this->settings_count[counter_index] = 0;
- }
- this->settings_count[counter_index] += count;
- this->settings_count_last_update = hrtime_sec;
+ this->_received_settings_counter.increment(count);
}
uint32_t
Http2ConnectionState::get_received_settings_count()
{
- return this->settings_count[0] + this->settings_count[1];
+ return this->_received_settings_counter.get_count();
}
void
Http2ConnectionState::increment_received_settings_frame_count()
{
- ink_hrtime hrtime_sec = Thread::get_hrtime() / HRTIME_SECOND;
- uint8_t counter_index = ((hrtime_sec % 60) >= 30);
-
- if ((hrtime_sec - 60) > this->settings_frame_count_last_update) {
- this->settings_frame_count[0] = 0;
- this->settings_frame_count[1] = 0;
- } else if (counter_index != ((this->settings_frame_count_last_update % 60)
>= 30)) {
- this->settings_frame_count[counter_index] = 0;
- }
- ++this->settings_frame_count[counter_index];
- this->settings_frame_count_last_update = hrtime_sec;
+ this->_received_settings_frame_counter.increment();
}
uint32_t
Http2ConnectionState::get_received_settings_frame_count()
{
- return this->settings_frame_count[0] + this->settings_frame_count[1];
+ return this->_received_settings_frame_counter.get_count();
}
void
Http2ConnectionState::increment_received_ping_frame_count()
{
- ink_hrtime hrtime_sec = Thread::get_hrtime() / HRTIME_SECOND;
- uint8_t counter_index = ((hrtime_sec % 60) >= 30);
-
- if ((hrtime_sec - 60) > this->ping_frame_count_last_update) {
- this->ping_frame_count[0] = 0;
- this->ping_frame_count[1] = 0;
- } else if (counter_index != ((this->ping_frame_count_last_update % 60) >=
30)) {
- this->ping_frame_count[counter_index] = 0;
- }
- ++this->ping_frame_count[counter_index];
- this->ping_frame_count_last_update = hrtime_sec;
+ this->_received_ping_frame_counter.increment();
}
uint32_t
Http2ConnectionState::get_received_ping_frame_count()
{
- return this->ping_frame_count[0] + this->ping_frame_count[1];
+ return this->_received_ping_frame_counter.get_count();
}
void
Http2ConnectionState::increment_received_priority_frame_count()
{
- ink_hrtime hrtime_sec = Thread::get_hrtime() / HRTIME_SECOND;
- uint8_t counter_index = ((hrtime_sec % 60) >= 30);
-
- if ((hrtime_sec - 60) > this->priority_frame_count_last_update) {
- this->priority_frame_count[0] = 0;
- this->priority_frame_count[1] = 0;
- } else if (counter_index != ((this->priority_frame_count_last_update % 60)
>= 30)) {
- this->priority_frame_count[counter_index] = 0;
- }
- ++this->priority_frame_count[counter_index];
- this->priority_frame_count_last_update = hrtime_sec;
+ this->_received_priority_frame_counter.increment();
}
uint32_t
Http2ConnectionState::get_received_priority_frame_count()
{
- return this->priority_frame_count[0] + this->priority_frame_count[1];
+ return this->_received_priority_frame_counter.get_count();
}
// Return min_concurrent_streams_in when current client streams number is
larger than max_active_streams_in.
diff --git a/proxy/http2/Http2ConnectionState.h
b/proxy/http2/Http2ConnectionState.h
index 9938483..01c80cf 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -28,6 +28,7 @@
#include "HPACK.h"
#include "Http2Stream.h"
#include "Http2DependencyTree.h"
+#include "Http2FrequencyCounter.h"
class Http2ClientSession;
@@ -358,18 +359,10 @@ private:
std::vector<size_t> _recent_rwnd_increment = {SIZE_MAX, SIZE_MAX, SIZE_MAX,
SIZE_MAX, SIZE_MAX};
int _recent_rwnd_increment_index = 0;
- // Counter for settings received within last 60 seconds
- // Each item holds a count for 30 seconds.
- uint16_t settings_count[2] = {0};
- ink_hrtime settings_count_last_update = 0;
- // Counters for frames received within last 60 seconds
- // Each item in an array holds a count for 30 seconds.
- uint16_t settings_frame_count[2] = {0};
- ink_hrtime settings_frame_count_last_update = 0;
- uint16_t ping_frame_count[2] = {0};
- ink_hrtime ping_frame_count_last_update = 0;
- uint16_t priority_frame_count[2] = {0};
- ink_hrtime priority_frame_count_last_update = 0;
+ Http2FrequencyCounter _received_settings_counter;
+ Http2FrequencyCounter _received_settings_frame_counter;
+ Http2FrequencyCounter _received_ping_frame_counter;
+ Http2FrequencyCounter _received_priority_frame_counter;
// NOTE: Id of stream which MUST receive CONTINUATION frame.
// - [RFC 7540] 6.2 HEADERS
diff --git a/proxy/http2/Http2FrequencyCounter.cc
b/proxy/http2/Http2FrequencyCounter.cc
new file mode 100644
index 0000000..dfe08b9
--- /dev/null
+++ b/proxy/http2/Http2FrequencyCounter.cc
@@ -0,0 +1,55 @@
+/** @file
+
+ Http2FrequencyCounter
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you 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 "Http2FrequencyCounter.h"
+
+void
+Http2FrequencyCounter::increment(uint16_t amount)
+{
+ ink_hrtime hrtime_sec = ink_hrtime_to_sec(Thread::get_hrtime());
+ uint8_t counter_index = ((hrtime_sec % 60) >= 30);
+ uint8_t last_index = ((this->_last_update % 60) >= 30);
+
+ if (hrtime_sec - this->_last_update > 60) {
+ this->_count[0] = 0;
+ this->_count[1] = 0;
+ } else if (hrtime_sec - this->_last_update > 30) {
+ if (counter_index == last_index) {
+ this->_count[0] = 0;
+ this->_count[1] = 0;
+ } else {
+ this->_count[counter_index] = 0;
+ }
+ } else if (counter_index != last_index) { // hrtime_sec - this->_last_update
is less than 30
+ this->_count[counter_index] = 0;
+ }
+
+ this->_count[counter_index] += amount;
+ this->_last_update = hrtime_sec;
+}
+
+uint32_t
+Http2FrequencyCounter::get_count()
+{
+ return this->_count[0] + this->_count[1];
+}
diff --git a/proxy/http2/Http2FrequencyCounter.h
b/proxy/http2/Http2FrequencyCounter.h
new file mode 100644
index 0000000..9d2ed73
--- /dev/null
+++ b/proxy/http2/Http2FrequencyCounter.h
@@ -0,0 +1,38 @@
+/** @file
+
+ Http2FrequencyCounter
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you 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.
+ */
+
+#pragma once
+
+#include <cstdint>
+#include "I_EventSystem.h"
+
+class Http2FrequencyCounter
+{
+public:
+ void increment(uint16_t amount = 1);
+ uint32_t get_count();
+
+protected:
+ uint16_t _count[2] = {0};
+ ink_hrtime _last_update = 0;
+};
diff --git a/proxy/http2/Makefile.am b/proxy/http2/Makefile.am
index ccc0d2d..3b33d36 100644
--- a/proxy/http2/Makefile.am
+++ b/proxy/http2/Makefile.am
@@ -45,6 +45,8 @@ libhttp2_a_SOURCES = \
Http2DebugNames.cc \
Http2DebugNames.h \
Http2DependencyTree.h \
+ Http2FrequencyCounter.h \
+ Http2FrequencyCounter.cc \
Http2Stream.cc \
Http2Stream.h \
Http2SessionAccept.cc \
@@ -57,10 +59,12 @@ endif
check_PROGRAMS = \
test_Http2DependencyTree \
+ test_Http2FrequencyCounter \
test_HPACK
TESTS = \
test_Http2DependencyTree \
+ test_Http2FrequencyCounter \
test_HPACK
test_Http2DependencyTree_LDADD = \
@@ -74,6 +78,19 @@ test_Http2DependencyTree_SOURCES = \
unit_tests/test_Http2DependencyTree.cc \
Http2DependencyTree.h
+test_Http2FrequencyCounter_LDADD = \
+ $(top_builddir)/iocore/eventsystem/libinkevent.a \
+ $(top_builddir)/src/tscore/libtscore.la \
+ $(top_builddir)/src/tscpp/util/libtscpputil.la
+
+test_Http2FrequencyCounter_CPPFLAGS = $(AM_CPPFLAGS)\
+ -I$(abs_top_srcdir)/tests/include
+
+test_Http2FrequencyCounter_SOURCES = \
+ unit_tests/test_Http2FrequencyCounter.cc \
+ Http2FrequencyCounter.cc \
+ Http2FequencyCounter.h
+
test_HPACK_LDADD = \
$(top_builddir)/proxy/hdrs/libhdrs.a \
$(top_builddir)/src/tscore/libtscore.la \
diff --git a/proxy/http2/unit_tests/test_Http2FrequencyCounter.cc
b/proxy/http2/unit_tests/test_Http2FrequencyCounter.cc
new file mode 100644
index 0000000..a1c0487
--- /dev/null
+++ b/proxy/http2/unit_tests/test_Http2FrequencyCounter.cc
@@ -0,0 +1,219 @@
+/** @file
+
+ Unit tests for Http2FrequencyCounter
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you 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.
+*/
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"
+
+#include "Http2FrequencyCounter.h"
+
+class TestHttp2FrequencyCounter : public Http2FrequencyCounter
+{
+public:
+ void
+ set_internal_state(ink_hrtime last_update_sec, uint16_t count_0, uint16_t
count_1)
+ {
+ this->_last_update = last_update_sec;
+ this->_count[0] = count_0;
+ this->_count[1] = count_1;
+ }
+};
+
+TEST_CASE("Http2FrequencyCounter_basic", "[http2][Http2FrequencyCounter]")
+{
+ TestHttp2FrequencyCounter counter;
+
+ SECTION("basic")
+ {
+ REQUIRE(counter.get_count() == 0);
+ counter.increment();
+ REQUIRE(counter.get_count() == 1);
+ counter.increment(2);
+ REQUIRE(counter.get_count() == 3);
+
+ counter.set_internal_state(ink_hrtime_to_sec(Thread::get_hrtime()) - 10,
1, 2);
+ REQUIRE(counter.get_count() == 3);
+ }
+
+ SECTION("Update at 0")
+ {
+ ink_hrtime now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ while (now % 60 != 0) {
+ sleep(1);
+ now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ }
+
+ counter.set_internal_state(now - 5, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 3);
+
+ counter.set_internal_state(now - 10, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 3);
+
+ counter.set_internal_state(now - 20, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 3);
+
+ counter.set_internal_state(now - 30, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 3);
+
+ counter.set_internal_state(now - 40, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 50, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 60, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 70, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+ }
+
+ SECTION("Update at 10")
+ {
+ ink_hrtime now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ while (now % 60 != 10) {
+ sleep(1);
+ now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ }
+
+ counter.set_internal_state(now - 5, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 4);
+
+ counter.set_internal_state(now - 10, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 4);
+
+ counter.set_internal_state(now - 20, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 3);
+
+ counter.set_internal_state(now - 30, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 3);
+
+ counter.set_internal_state(now - 40, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 3);
+
+ counter.set_internal_state(now - 50, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 60, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 70, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+ }
+
+ SECTION("Update at 30")
+ {
+ ink_hrtime now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ while (now % 60 != 30) {
+ sleep(1);
+ now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ }
+
+ counter.set_internal_state(now - 5, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 2);
+
+ counter.set_internal_state(now - 10, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 2);
+
+ counter.set_internal_state(now - 20, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 2);
+
+ counter.set_internal_state(now - 30, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 2);
+
+ counter.set_internal_state(now - 40, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 50, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 60, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 70, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+ }
+
+ SECTION("Update at 40")
+ {
+ ink_hrtime now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ while (now % 60 != 40) {
+ sleep(1);
+ now = ink_hrtime_to_sec(Thread::get_hrtime_updated());
+ }
+
+ counter.set_internal_state(now - 5, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 4);
+
+ counter.set_internal_state(now - 10, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 4);
+
+ counter.set_internal_state(now - 20, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 2);
+
+ counter.set_internal_state(now - 30, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 2);
+
+ counter.set_internal_state(now - 40, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 2);
+
+ counter.set_internal_state(now - 50, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 60, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+
+ counter.set_internal_state(now - 70, 1, 2);
+ counter.increment();
+ CHECK(counter.get_count() == 1);
+ }
+}
