You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ch...@apache.org on 2020/10/23 14:25:12 UTC
[qpid-dispatch] branch master updated: DISPATCH-1751: Fix 32-bit
self test receiving unexpected incoming-capacity
This is an automated email from the ASF dual-hosted git repository.
chug pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git
The following commit(s) were added to refs/heads/master by this push:
new 5371dfb DISPATCH-1751: Fix 32-bit self test receiving unexpected incoming-capacity
5371dfb is described below
commit 5371dfbedc60fd6c429c237d7053c3e15f0d25fe
Author: Chuck Rolke <ch...@apache.org>
AuthorDate: Fri Oct 23 09:52:23 2020 -0400
DISPATCH-1751: Fix 32-bit self test receiving unexpected incoming-capacity
Dispatch listener config defines a 'maxFrameSize' and a 'maxSessionFrames'.
The idea is to have the 'maxFrameSize' go out in AMQP Open frames as
'max-frame-size' and the 'maxSessionFrames' go out in AMQP Begin frames
as 'incoming-capacity'.
To accomplish this Proton accepts a maxFrameSize and a capacity.
For Dispatch to get Proton to emit the 'incoming-capacity' then Dispatch
must specify 'capacity' as the product of 'maxFrameSize' and
'maxSessionFrames'. Furthermore, Proton specifies the capacity value
as a system-dependent 'size_t' value type.
For 64-bit systems the chosen defaults work just fine although the
capacity is a huge 30+ Terabytes. For 32-bit systems the 30+ Tb number
does not work since that does not fit into a 32-bit size_t. Dispatch
uses a smaller fallback maximum for 32-bit systems.
The self test was failing on 32-bit systems because the self test did not
account for the 32-bit smaller fallback maximum.
This patch fixes the self test to account for the fallback maximum on
32-bit systems.
This patch also rewrites for readability the 32-bit/64-bit capacity
calculations in the mission C code. It also computes capacity values
using 64-bit numbers even on 32-bit systems to avoid size_t overflow
truncation errors.
This patch does *not* use a different strategy for driving Proton to
use simpler calculation methods when essentially unlimited capacity
values are in effect.
Future work in this area might address some of the issues brought up
during Pull Request reviews. Thank you to the reviewers!
* Modify Proton to accept an 'incoming-capacity' value instead of or
in addition to 'capacity'.
* Modify Dispatch not to set a capacity at all in Proton when no flow
control in effect.
This closes #890
---
src/connection_manager.c | 44 ++++++++++++++++++++-------------
tests/system_tests_protocol_settings.py | 28 ++++++++++++++++-----
2 files changed, 49 insertions(+), 23 deletions(-)
diff --git a/src/connection_manager.c b/src/connection_manager.c
index aa95417..65cdfb6 100644
--- a/src/connection_manager.c
+++ b/src/connection_manager.c
@@ -438,25 +438,35 @@ static qd_error_t load_server_config(qd_dispatch_t *qd, qd_server_config_t *conf
config->max_frame_size = QD_AMQP_MIN_MAX_FRAME_SIZE;
//
- // Given session frame count and max frame size compute session incoming_capacity
+ // Given session frame count and max frame size, compute session incoming_capacity
+ // On 64-bit systems the capacity has no practial limit.
+ // On 32-bit systems the largest default capacity is half the process address space.
//
- if (ssn_frames == 0)
- config->incoming_capacity = (sizeof(size_t) < 8) ? 0x7FFFFFFFLL : 0x7FFFFFFFLL * config->max_frame_size;
- else {
- uint64_t mfs = (uint64_t) config->max_frame_size;
- uint64_t trial_ic = ssn_frames * mfs;
- uint64_t limit = (sizeof(size_t) < 8) ? (1ll << 31) - 1 : 0;
- if (limit == 0 || trial_ic < limit) {
- // Silently promote incoming capacity of zero to one
- config->incoming_capacity =
- (trial_ic < QD_AMQP_MIN_MAX_FRAME_SIZE ? QD_AMQP_MIN_MAX_FRAME_SIZE : trial_ic);
+ bool is_64bit = sizeof(size_t) == 8;
+#define MAX_32BIT_CAPACITY ((size_t)(2147483647))
+ if (ssn_frames == 0) {
+ config->incoming_capacity = is_64bit ? MAX_32BIT_CAPACITY * (size_t)config->max_frame_size : MAX_32BIT_CAPACITY;
+ } else {
+ // Limited incoming frames.
+ if (is_64bit) {
+ // Specify this to proton by setting capacity to be
+ // the product (max_frame_size * ssn_frames).
+ config->incoming_capacity = (size_t)config->max_frame_size * (size_t)ssn_frames;
} else {
- config->incoming_capacity = limit;
- uint64_t computed_ssn_frames = limit / mfs;
- qd_log(qd->connection_manager->log_source, QD_LOG_WARNING,
- "Server configuation for I/O adapter entity name:'%s', host:'%s', port:'%s', "
- "requested maxSessionFrames truncated from %"PRId64" to %"PRId64,
- config->name, config->host, config->port, ssn_frames, computed_ssn_frames);
+ // 32-bit systems have an upper bound to the capacity
+ uint64_t max_32bit_capacity = (uint64_t)MAX_32BIT_CAPACITY;
+ uint64_t capacity = (uint64_t)config->max_frame_size * (uint64_t)ssn_frames;
+ if (capacity <= max_32bit_capacity) {
+ config->incoming_capacity = (size_t)capacity;
+ } else {
+ config->incoming_capacity = MAX_32BIT_CAPACITY;
+ uint64_t actual_frames = max_32bit_capacity / (uint64_t)config->max_frame_size;
+
+ qd_log(qd->connection_manager->log_source, QD_LOG_WARNING,
+ "Server configuation for I/O adapter entity name:'%s', host:'%s', port:'%s', "
+ "requested maxSessionFrames truncated from %"PRId64" to %"PRId64,
+ config->name, config->host, config->port, ssn_frames, actual_frames);
+ }
}
}
diff --git a/tests/system_tests_protocol_settings.py b/tests/system_tests_protocol_settings.py
index 546ffe6..454f0c0 100644
--- a/tests/system_tests_protocol_settings.py
+++ b/tests/system_tests_protocol_settings.py
@@ -26,6 +26,7 @@ from system_test import TestCase, Qdrouterd, main_module
from system_test import unittest
from proton.utils import BlockingConnection
import subprocess
+import sys
class MaxFrameMaxSessionFramesTest(TestCase):
"""System tests setting proton negotiated size max-frame-size and incoming-window"""
@@ -234,8 +235,13 @@ class MaxSessionFramesDefaultTest(TestCase):
# if frame size not set then a default is used
self.assertTrue(" max-frame-size=16384" in open_lines[0])
begin_lines = [s for s in log_lines if "-> @begin" in s]
- # incoming-window is defaulted to 2^31-1 (Proton default)
- self.assertTrue(" incoming-window=2147483647," in begin_lines[0])
+ # incoming-window should be 2^31-1 (64-bit) or
+ # (2^31-1) / max-frame-size (32-bit)
+ is_64bits = sys.maxsize > 2 ** 32
+ expected = " incoming-window=2147483647," if is_64bits else \
+ (" incoming-window=%d," % int(2147483647 / 16384))
+ self.assertTrue(expected in begin_lines[0],
+ "Expected:'%s' not found in '%s'" % (expected, begin_lines[0]))
class MaxFrameMaxSessionFramesZeroTest(TestCase):
@@ -270,8 +276,13 @@ class MaxFrameMaxSessionFramesZeroTest(TestCase):
# max-frame gets set to protocol min
self.assertTrue(' max-frame-size=512,' in open_lines[0])
begin_lines = [s for s in log_lines if "-> @begin" in s]
- # incoming-window is defaulted to 2^31-1 (Proton default)
- self.assertTrue(" incoming-window=2147483647," in begin_lines[0])
+ # incoming-window should be 2^31-1 (64-bit) or
+ # (2^31-1) / max-frame-size (32-bit)
+ is_64bits = sys.maxsize > 2 ** 32
+ expected = " incoming-window=2147483647," if is_64bits else \
+ (" incoming-window=%d," % int(2147483647 / 512))
+ self.assertTrue(expected in begin_lines[0],
+ "Expected:'%s' not found in '%s'" % (expected, begin_lines[0]))
class ConnectorSettingsDefaultTest(TestCase):
@@ -323,8 +334,13 @@ class ConnectorSettingsDefaultTest(TestCase):
self.assertTrue(' max-frame-size=16384,' in open_lines[0])
self.assertTrue(' channel-max=32767,' in open_lines[0])
begin_lines = [s for s in log_lines if "<- @begin" in s]
- # defaults
- self.assertTrue(" incoming-window=2147483647," in begin_lines[0])
+ # incoming-window should be 2^31-1 (64-bit) or
+ # (2^31-1) / max-frame-size (32-bit)
+ is_64bits = sys.maxsize > 2 ** 32
+ expected = " incoming-window=2147483647," if is_64bits else \
+ (" incoming-window=%d," % int(2147483647 / 16384))
+ self.assertTrue(expected in begin_lines[0],
+ "Expected:'%s' not found in '%s'" % (expected, begin_lines[0]))
class ConnectorSettingsNondefaultTest(TestCase):
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org