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 <c...@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-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org