You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kg...@apache.org on 2017/09/28 13:57:43 UTC
qpid-dispatch git commit: DISPATCH-813: change lifecycle of link
route to remove reference count
Repository: qpid-dispatch
Updated Branches:
refs/heads/master e9b6de644 -> 077d053ff
DISPATCH-813: change lifecycle of link route to remove reference count
Remove the ref counting when adding/removing link route patterns and
instead simply add the pattern when the address is first added to the
hash table, and remove it when the address is removed.
Technique proposed by Ted Ross.
This closes #202
Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/077d053f
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/077d053f
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/077d053f
Branch: refs/heads/master
Commit: 077d053ff1aeb5ef2f50d04cbab8ef44ee0be763
Parents: e9b6de6
Author: Kenneth Giusti <kg...@apache.org>
Authored: Wed Sep 27 16:12:05 2017 -0400
Committer: Kenneth Giusti <kg...@apache.org>
Committed: Thu Sep 28 09:56:33 2017 -0400
----------------------------------------------------------------------
src/router_core/route_control.c | 45 ++++++++++++------------------
src/router_core/route_tables.c | 25 ++++++-----------
src/router_core/router_core.c | 6 ++++
src/router_core/router_core_private.h | 1 -
tests/system_tests_link_routes.py | 40 ++++++++++++++++----------
5 files changed, 59 insertions(+), 58 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/077d053f/src/router_core/route_control.c
----------------------------------------------------------------------
diff --git a/src/router_core/route_control.c b/src/router_core/route_control.c
index fa5c7d4..90f931b 100644
--- a/src/router_core/route_control.c
+++ b/src/router_core/route_control.c
@@ -307,7 +307,6 @@ qdr_link_route_t *qdr_route_add_link_route_CT(qdr_core_t *core,
lr->is_prefix = is_prefix;
lr->pattern = pattern;
-
//
// Add the address to the routing hash table and map it as a pattern in the
// wildcard pattern parse tree
@@ -320,8 +319,9 @@ qdr_link_route_t *qdr_route_add_link_route_CT(qdr_core_t *core,
lr->addr = qdr_address_CT(core, treatment);
DEQ_INSERT_TAIL(core->addrs, lr->addr);
qd_hash_insert(core->addr_hash, a_iter, lr->addr, &lr->addr->hash_handle);
+ qdr_link_route_map_pattern_CT(core, a_iter, lr->addr);
}
- qdr_link_route_map_pattern_CT(core, a_iter, lr->addr);
+
qd_iterator_free(a_iter);
free(addr_hash);
}
@@ -369,19 +369,8 @@ void qdr_route_del_link_route_CT(qdr_core_t *core, qdr_link_route_t *lr)
}
//
- // Pull the pattern from the parse tree
- //
- {
- char *addr = qdr_link_route_pattern_to_address(lr->pattern, lr->dir);
- qd_iterator_t *iter = qd_iterator_string(addr, ITER_VIEW_ALL);
- qdr_link_route_unmap_pattern_CT(core, iter);
- qd_iterator_free(iter);
- free(addr);
- }
-
- //
// Disassociate the link route from its address. Check to see if the address
- // should be removed.
+ // (and its associated pattern) should be removed.
//
qdr_address_t *addr = lr->addr;
if (addr && --addr->ref_count == 0)
@@ -563,6 +552,9 @@ void qdr_route_connection_closed_CT(qdr_core_t *core, qdr_connection_t *conn)
}
+// add the link route address pattern to the lookup tree
+// address is the hashed address, which includes 'C','D','E', or 'F' classifier
+// in the first char. The pattern follows in the second char.
void qdr_link_route_map_pattern_CT(qdr_core_t *core, qd_iterator_t *address, qdr_address_t *addr)
{
qd_direction_t dir;
@@ -573,20 +565,21 @@ void qdr_link_route_map_pattern_CT(qdr_core_t *core, qd_iterator_t *address, qdr
bool found = qd_parse_tree_get_pattern(core->link_route_tree[dir], iter, (void **)&other_addr);
if (!found) {
qd_parse_tree_add_pattern(core->link_route_tree[dir], iter, addr);
- addr->ref_count++;
} else {
- // already mapped
- if (other_addr != addr)
- qd_log(core->log, QD_LOG_CRITICAL, "Link route %s not correctly mapped",
- pattern);
+ // the pattern is mapped once when the address is added to the hash
+ // table. It should not be mapped twice
+ qd_log(core->log, QD_LOG_CRITICAL, "Link route %s mapped redundantly!",
+ pattern);
}
- addr->map_count++;
qd_iterator_free(iter);
free(pattern);
}
+// remove the link route address pattern from the lookup tree.
+// address is the hashed address, which includes 'C','D','E', or 'F' classifier
+// in the first char. The pattern follows in the second char.
void qdr_link_route_unmap_pattern_CT(qdr_core_t *core, qd_iterator_t *address)
{
qd_direction_t dir;
@@ -595,15 +588,13 @@ void qdr_link_route_unmap_pattern_CT(qdr_core_t *core, qd_iterator_t *address)
qdr_address_t *addr;
bool found = qd_parse_tree_get_pattern(core->link_route_tree[dir], iter, (void **)&addr);
if (found) {
- assert(addr->map_count > 0);
- addr->map_count--;
- if (addr->map_count == 0) {
- qd_parse_tree_remove_pattern(core->link_route_tree[dir], iter);
- addr->ref_count--;
- }
- } else
+ qd_parse_tree_remove_pattern(core->link_route_tree[dir], iter);
+ } else {
+ // expected that the pattern is removed when the address is deleted.
+ // Attempting to remove it twice is unexpected
qd_log(core->log, QD_LOG_CRITICAL, "link route pattern ummap: Pattern '%s' not found",
pattern);
+ }
qd_iterator_free(iter);
free(pattern);
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/077d053f/src/router_core/route_tables.c
----------------------------------------------------------------------
diff --git a/src/router_core/route_tables.c b/src/router_core/route_tables.c
index 855ec45..dd2472f 100644
--- a/src/router_core/route_tables.c
+++ b/src/router_core/route_tables.c
@@ -572,8 +572,6 @@ static void qdr_map_destination_CT(qdr_core_t *core, qdr_action_t *action, bool
qd_iterator_t *iter = address->iterator;
qdr_address_t *addr = 0;
- const char prefix = (char) qd_iterator_octet(iter);
- qd_iterator_reset(iter);
qd_hash_retrieve(core->addr_hash, iter, (void**) &addr);
if (!addr) {
@@ -581,11 +579,14 @@ static void qdr_map_destination_CT(qdr_core_t *core, qdr_action_t *action, bool
qd_hash_insert(core->addr_hash, iter, addr, &addr->hash_handle);
DEQ_ITEM_INIT(addr);
DEQ_INSERT_TAIL(core->addrs, addr);
- }
-
- if (QDR_IS_LINK_ROUTE(prefix)) {
- // add the link route pattern to the wildcard address parse tree
- qdr_link_route_map_pattern_CT(core, iter, addr);
+ // if the address is a link route, add the pattern to the wildcard
+ // address parse tree
+ {
+ const char *a_str = (const char *)qd_hash_key_by_handle(addr->hash_handle);
+ if (QDR_IS_LINK_ROUTE(a_str[0])) {
+ qdr_link_route_map_pattern_CT(core, iter, addr);
+ }
+ }
}
qdr_node_t *rnode = core->routers_by_mask_bit[router_maskbit];
@@ -623,21 +624,13 @@ static void qdr_unmap_destination_CT(qdr_core_t *core, qdr_action_t *action, boo
qdr_node_t *rnode = core->routers_by_mask_bit[router_maskbit];
qd_iterator_t *iter = address->iterator;
qdr_address_t *addr = 0;
- const char prefix = (char) qd_iterator_octet(iter);
- qd_iterator_reset(iter);
qd_hash_retrieve(core->addr_hash, iter, (void**) &addr);
-
if (!addr) {
qd_log(core->log, QD_LOG_CRITICAL, "unmap_destination: Address not found");
break;
}
-
- if (QDR_IS_LINK_ROUTE(prefix)) {
- // pull it from the wildcard address parse tree
- qdr_link_route_unmap_pattern_CT(core, iter);
- }
-
+
qd_bitmask_clear_bit(addr->rnodes, router_maskbit);
rnode->ref_count--;
addr->cost_epoch--;
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/077d053f/src/router_core/router_core.c
----------------------------------------------------------------------
diff --git a/src/router_core/router_core.c b/src/router_core/router_core.c
index 3734c92..c8b82fa 100644
--- a/src/router_core/router_core.c
+++ b/src/router_core/router_core.c
@@ -338,6 +338,12 @@ void qdr_core_remove_address(qdr_core_t *core, qdr_address_t *addr)
// Remove the address from the list, hash index, and parse tree
DEQ_REMOVE(core->addrs, addr);
if (addr->hash_handle) {
+ const char *a_str = (const char *)qd_hash_key_by_handle(addr->hash_handle);
+ if (QDR_IS_LINK_ROUTE(a_str[0])) {
+ qd_iterator_t *iter = qd_iterator_string(a_str, ITER_VIEW_ALL);
+ qdr_link_route_unmap_pattern_CT(core, iter);
+ qd_iterator_free(iter);
+ }
qd_hash_remove_by_handle(core->addr_hash, addr->hash_handle);
qd_hash_handle_free(addr->hash_handle);
}
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/077d053f/src/router_core/router_core_private.h
----------------------------------------------------------------------
diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h
index 2f14b4c..6e61db7 100644
--- a/src/router_core/router_core_private.h
+++ b/src/router_core/router_core_private.h
@@ -445,7 +445,6 @@ struct qdr_address_t {
qd_address_treatment_t treatment;
qdr_forwarder_t *forwarder;
int ref_count; ///< Number of link-routes + auto-links referencing this address
- int map_count; ///< parse tree map/unmap operations for link route patterns
bool block_deletion;
bool local;
uint32_t tracked_deliveries;
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/077d053f/tests/system_tests_link_routes.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_link_routes.py b/tests/system_tests_link_routes.py
index d8a1ec3..0cdc22b 100644
--- a/tests/system_tests_link_routes.py
+++ b/tests/system_tests_link_routes.py
@@ -93,6 +93,7 @@ class LinkRouteTest(TestCase):
])
router('B',
[
+ # Listener for clients, note that the tests assume this listener is first in this list:
('listener', {'role': 'normal', 'host': '0.0.0.0', 'port': b_listener_port, 'saslMechanisms': 'ANONYMOUS'}),
('listener', {'name': 'test-tag', 'role': 'route-container', 'host': '0.0.0.0', 'port': test_tag_listener_port, 'saslMechanisms': 'ANONYMOUS'}),
@@ -123,7 +124,9 @@ class LinkRouteTest(TestCase):
)
router('C',
[
- # The client will exclusively use the following listener to connect to QDR.C
+ # The client will exclusively use the following listener to
+ # connect to QDR.C, the tests assume this is the first entry
+ # in the list
('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': cls.tester.get_port(), 'saslMechanisms': 'ANONYMOUS'}),
('listener', {'host': '0.0.0.0', 'role': 'inter-router', 'port': c_listener_port, 'saslMechanisms': 'ANONYMOUS'}),
# The dot(.) at the end is ignored by the address hashing scheme.
@@ -399,8 +402,9 @@ class LinkRouteTest(TestCase):
blocking_connection.close()
- def _link_route_pattern_match(self, connect_host, include_host,
- exclude_host, test_address):
+ def _link_route_pattern_match(self, connect_node, include_host,
+ exclude_host, test_address,
+ expected_pattern):
"""
This helper function ensures that messages sent to 'test_address' pass
through 'include_host', and are *not* routed to 'exclude_host'
@@ -421,9 +425,13 @@ class LinkRouteTest(TestCase):
type='org.apache.qpid.dispatch.router.address',
name=route)
- # Connect to 'connect_host' and send message to 'address'
+ # wait until the host we're connecting to gets its next hop for the
+ # pattern we're connecting to
+ connect_node.wait_address(expected_pattern, remotes=1, delay=0.1)
- blocking_connection = BlockingConnection(connect_host)
+ # Connect to 'connect_node' and send message to 'address'
+
+ blocking_connection = BlockingConnection(connect_node.addresses[0])
blocking_receiver = blocking_connection.create_receiver(address=test_address)
blocking_sender = blocking_connection.create_sender(address=test_address,
options=AtMostOnce())
@@ -469,24 +477,28 @@ class LinkRouteTest(TestCase):
"""
qdr_A = self.routers[0].addresses[0]
qdr_D = self.routers[3].addresses[0]
- qdr_C = self.routers[2].addresses[0]
+ qdr_C = self.routers[2] # note: the node, not the address!
- self._link_route_pattern_match(connect_host=qdr_C,
+ self._link_route_pattern_match(connect_node=qdr_C,
include_host=qdr_A,
exclude_host=qdr_D,
- test_address='a.notD.toA')
- self._link_route_pattern_match(connect_host=qdr_C,
+ test_address='a.notD.toA',
+ expected_pattern='a.*.toA.#')
+ self._link_route_pattern_match(connect_node=qdr_C,
include_host=qdr_D,
exclude_host=qdr_A,
- test_address='a.notA.toD')
- self._link_route_pattern_match(connect_host=qdr_C,
+ test_address='a.notA.toD',
+ expected_pattern='a.*.toD.#')
+ self._link_route_pattern_match(connect_node=qdr_C,
include_host=qdr_A,
exclude_host=qdr_D,
- test_address='a.toD.toA.xyz')
- self._link_route_pattern_match(connect_host=qdr_C,
+ test_address='a.toD.toA.xyz',
+ expected_pattern='a.*.toA.#')
+ self._link_route_pattern_match(connect_node=qdr_C,
include_host=qdr_D,
exclude_host=qdr_A,
- test_address='a.toA.toD.abc')
+ test_address='a.toA.toD.abc',
+ expected_pattern='a.*.toD.#')
def test_custom_annotations_match(self):
"""
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org