You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by gm...@apache.org on 2020/05/06 20:31:45 UTC
[qpid-dispatch] branch master updated: DISPATCH-1637: Added a
hashtable to store names of link routes,
auto links and address configs. This closes #732.
This is an automated email from the ASF dual-hosted git repository.
gmurthy 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 936d679 DISPATCH-1637: Added a hashtable to store names of link routes, auto links and address configs. This closes #732.
936d679 is described below
commit 936d679f754639895dccccf85b665651ed4d73f3
Author: Ganesh Murthy <gm...@apache.org>
AuthorDate: Tue May 5 08:56:06 2020 -0400
DISPATCH-1637: Added a hashtable to store names of link routes, auto links and address configs. This closes #732.
---
src/hash.c | 9 +++
src/router_core/agent_config_address.c | 24 +++++--
src/router_core/agent_config_auto_link.c | 15 ++--
src/router_core/agent_config_link_route.c | 15 ++--
src/router_core/route_control.c | 36 ++++++++++
src/router_core/route_tables.c | 5 +-
src/router_core/router_core.c | 11 +++
src/router_core/router_core_private.h | 6 ++
tests/system_tests_autolinks.py | 114 ++++++++++++++++++++++++++++++
9 files changed, 217 insertions(+), 18 deletions(-)
diff --git a/src/hash.c b/src/hash.c
index 03acfdf..89c2016 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -267,6 +267,11 @@ void qd_hash_retrieve_prefix_const(qd_hash_t *h, qd_iterator_t *iter, const void
qd_error_t qd_hash_retrieve(qd_hash_t *h, qd_iterator_t *key, void **val)
{
+ if (!key) {
+ *val = 0;
+ return QD_ERROR_NONE;
+ }
+
qd_hash_item_t *item = qd_hash_internal_retrieve(h, key);
if (item)
*val = item->v.val;
@@ -322,10 +327,14 @@ const unsigned char *qd_hash_key_by_handle(const qd_hash_handle_t *handle)
qd_error_t qd_hash_remove_by_handle(qd_hash_t *h, qd_hash_handle_t *handle)
{
+ if (!handle)
+ return QD_ERROR_NONE;
+
unsigned char *key = 0;
qd_error_t error = qd_hash_remove_by_handle2(h, handle, &key);
if (key)
free(key);
+
return error;
}
diff --git a/src/router_core/agent_config_address.c b/src/router_core/agent_config_address.c
index b25779a..c04267e 100644
--- a/src/router_core/agent_config_address.c
+++ b/src/router_core/agent_config_address.c
@@ -49,6 +49,7 @@ const char *qdr_config_address_columns[] =
0};
const char *CONFIG_ADDRESS_TYPE = "org.apache.qpid.dispatch.router.config.address";
+const char CONFIG_ADDRESS_PREFIX = 'C';
static void qdr_config_address_insert_column_CT(qdr_address_config_t *addr, int col, qd_composed_field_t *body, bool as_map)
{
@@ -340,13 +341,15 @@ void qdra_config_address_create_CT(qdr_core_t *core,
while (true) {
//
- // Ensure there isn't a duplicate name and that the body is a map
+ // Ensure there isn't a duplicate name
//
- qdr_address_config_t *addr = DEQ_HEAD(core->addr_config);
- while (addr) {
- if (name && addr->name && qd_iterator_equal(name, (const unsigned char*) addr->name))
- break;
- addr = DEQ_NEXT(addr);
+ qdr_address_config_t *addr = 0;
+ if (name) {
+ qd_iterator_view_t iter_view = qd_iterator_get_view(name);
+ qd_iterator_annotate_prefix(name, CONFIG_ADDRESS_PREFIX);
+ qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH);
+ qd_hash_retrieve(core->addr_lr_al_hash, name, (void**) &addr);
+ qd_iterator_reset_view(name, iter_view);
}
if (!!addr) {
@@ -356,6 +359,7 @@ void qdra_config_address_create_CT(qdr_core_t *core,
break;
}
+ // Ensure that the body is a map
if (!qd_parse_is_map(in_body)) {
query->status = QD_AMQP_BAD_REQUEST;
query->status.description = "Body of request must be a map";
@@ -476,8 +480,14 @@ void qdra_config_address_create_CT(qdr_core_t *core,
qd_iterator_reset_view(iter, ITER_VIEW_ALL);
qd_parse_tree_add_pattern(core->addr_parse_tree, iter, addr);
- DEQ_INSERT_TAIL(core->addr_config, addr);
+ DEQ_INSERT_TAIL(core->addr_config, addr);
+ if (name) {
+ qd_iterator_view_t iter_view = qd_iterator_get_view(name);
+ qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH);
+ qd_hash_insert(core->addr_lr_al_hash, name, addr, &addr->hash_handle);
+ qd_iterator_reset_view(name, iter_view);
+ }
//
// Compose the result map for the response.
//
diff --git a/src/router_core/agent_config_auto_link.c b/src/router_core/agent_config_auto_link.c
index db2f941..f576221 100644
--- a/src/router_core/agent_config_auto_link.c
+++ b/src/router_core/agent_config_auto_link.c
@@ -60,6 +60,7 @@ const char *qdr_config_auto_link_columns[] =
0};
const char *CONFIG_AUTOLINK_TYPE = "org.apache.qpid.dispatch.router.config.autoLink";
+const char CONFIG_AUTO_LINK_PREFIX = 'A';
static void qdr_config_auto_link_insert_column_CT(qdr_auto_link_t *al, int col, qd_composed_field_t *body, bool as_map)
{
@@ -374,13 +375,17 @@ void qdra_config_auto_link_create_CT(qdr_core_t *core,
//
// Ensure there isn't a duplicate name and that the body is a map
//
- qdr_auto_link_t *al = DEQ_HEAD(core->auto_links);
- while (al) {
- if (name && al->name && qd_iterator_equal(name, (const unsigned char*) al->name))
- break;
- al = DEQ_NEXT(al);
+ qdr_auto_link_t *al = 0;
+
+ if (name) {
+ qd_iterator_view_t iter_view = qd_iterator_get_view(name);
+ qd_iterator_annotate_prefix(name, CONFIG_AUTO_LINK_PREFIX);
+ qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH);
+ qd_hash_retrieve(core->addr_lr_al_hash, name, (void**) &al);
+ qd_iterator_reset_view(name, iter_view);
}
+
if (!!al) {
query->status = QD_AMQP_BAD_REQUEST;
query->status.description = "Name conflicts with an existing entity";
diff --git a/src/router_core/agent_config_link_route.c b/src/router_core/agent_config_link_route.c
index c04daa2..a427194 100644
--- a/src/router_core/agent_config_link_route.c
+++ b/src/router_core/agent_config_link_route.c
@@ -55,6 +55,7 @@ const char *qdr_config_link_route_columns[] =
0};
const char *CONFIG_LINKROUTE_TYPE = "org.apache.qpid.dispatch.router.config.linkRoute";
+const char CONFIG_LINK_ROUTE_PREFIX = 'L';
const qd_amqp_error_t QD_AMQP_NAME_IDENTITY_MISSING = { 400, "No name or identity provided" };
@@ -396,13 +397,17 @@ void qdra_config_link_route_create_CT(qdr_core_t *core,
//
// Ensure there isn't a duplicate name and that the body is a map
//
- qdr_link_route_t *lr = DEQ_HEAD(core->link_routes);
- while (lr) {
- if (name && lr->name && qd_iterator_equal(name, (const unsigned char*) lr->name))
- break;
- lr = DEQ_NEXT(lr);
+ qdr_link_route_t *lr = 0;
+
+ if (name) {
+ qd_iterator_view_t iter_view = qd_iterator_get_view(name);
+ qd_iterator_annotate_prefix(name, CONFIG_LINK_ROUTE_PREFIX);
+ qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH);
+ qd_hash_retrieve(core->addr_lr_al_hash, name, (void**) &lr);
+ qd_iterator_reset_view(name, iter_view);
}
+
if (!!lr) {
query->status = QD_AMQP_BAD_REQUEST;
query->status.description = "Name conflicts with an existing entity";
diff --git a/src/router_core/route_control.c b/src/router_core/route_control.c
index 68de4a0..f6915b1 100644
--- a/src/router_core/route_control.c
+++ b/src/router_core/route_control.c
@@ -378,9 +378,22 @@ qdr_link_route_t *qdr_route_add_link_route_CT(qdr_core_t *core,
}
//
+ // If a name was provided, use that as the key to insert the this link route name into the hashtable so
+ // we can quickly find it later.
+ //
+ if (name) {
+ qd_iterator_view_t iter_view = qd_iterator_get_view(name);
+ qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH);
+ qd_hash_insert(core->addr_lr_al_hash, name, lr, &lr->hash_handle);
+ qd_iterator_reset_view(name, iter_view);
+ }
+
+ //
// Add the link route to the core list
//
DEQ_INSERT_TAIL(core->link_routes, lr);
+
+
qd_log(core->log, QD_LOG_TRACE, "Link route %spattern added: pattern=%s name=%s",
is_prefix ? "prefix " : "", lr->pattern, lr->name);
@@ -441,6 +454,12 @@ void qdr_route_del_link_route_CT(qdr_core_t *core, qdr_link_route_t *lr)
}
}
+ if (lr->hash_handle) {
+ qd_hash_remove_by_handle(core->addr_lr_al_hash, lr->hash_handle);
+ qd_hash_handle_free(lr->hash_handle);
+ lr->hash_handle = 0;
+ }
+
//
// Remove the link route from the core list.
//
@@ -520,6 +539,17 @@ qdr_auto_link_t *qdr_route_add_auto_link_CT(qdr_core_t *core,
}
//
+ // If a name was provided, use that as the key to insert the this auto link name into the hashtable so
+ // we can quickly find it later.
+ //
+ if (name) {
+ qd_iterator_view_t iter_view = qd_iterator_get_view(name);
+ qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH);
+ qd_hash_insert(core->addr_lr_al_hash, name, al, &al->hash_handle);
+ qd_iterator_reset_view(name, iter_view);
+ }
+
+ //
// Add the auto_link to the core list
//
DEQ_INSERT_TAIL(core->auto_links, al);
@@ -543,6 +573,12 @@ void qdr_route_del_auto_link_CT(qdr_core_t *core, qdr_auto_link_t *al)
}
}
+ if (al->hash_handle) {
+ qd_hash_remove_by_handle(core->addr_lr_al_hash, al->hash_handle);
+ qd_hash_handle_free(al->hash_handle);
+ al->hash_handle = 0;
+ }
+
//
// Remove the auto link from the core list.
//
diff --git a/src/router_core/route_tables.c b/src/router_core/route_tables.c
index d6fdb15..7c55964 100644
--- a/src/router_core/route_tables.c
+++ b/src/router_core/route_tables.c
@@ -219,7 +219,10 @@ void qdr_route_table_setup_CT(qdr_core_t *core)
{
DEQ_INIT(core->addrs);
DEQ_INIT(core->routers);
- core->addr_hash = qd_hash(12, 32, 0);
+ core->addr_hash = qd_hash(12, 32, 0);
+ core->addr_lr_al_hash = qd_hash(12, 32, 0);
+
+
core->conn_id_hash = qd_hash(6, 4, 0);
core->cost_epoch = 1;
core->addr_parse_tree = qd_parse_tree_new(QD_PARSE_TREE_ADDRESS);
diff --git a/src/router_core/router_core.c b/src/router_core/router_core.c
index 1a01ed3..ec8e0ad 100644
--- a/src/router_core/router_core.c
+++ b/src/router_core/router_core.c
@@ -149,7 +149,10 @@ void qdr_core_free(qdr_core_t *core)
while ( (addr_config = DEQ_HEAD(core->addr_config))) {
qdr_core_remove_address_config(core, addr_config);
}
+
qd_hash_free(core->addr_hash);
+ qd_hash_free(core->addr_lr_al_hash);
+
qd_parse_tree_free(core->addr_parse_tree);
qd_parse_tree_free(core->link_route_tree[QD_INCOMING]);
qd_parse_tree_free(core->link_route_tree[QD_OUTGOING]);
@@ -456,6 +459,7 @@ void qdr_core_delete_link_route(qdr_core_t *core, qdr_link_route_t *lr)
free(lr->del_prefix);
free(lr->name);
free(lr->pattern);
+ qd_hash_handle_free(lr->hash_handle);
free_qdr_link_route_t(lr);
}
@@ -472,6 +476,7 @@ void qdr_core_delete_auto_link(qdr_core_t *core, qdr_auto_link_t *al)
free(al->name);
free(al->external_addr);
+ qd_hash_handle_free(al->hash_handle);
qdr_core_timer_free_CT(core, al->retry_timer);
free_qdr_auto_link_t(al);
}
@@ -480,6 +485,7 @@ static void free_address_config(qdr_address_config_t *addr)
{
free(addr->name);
free(addr->pattern);
+ qd_hash_handle_free(addr->hash_handle);
free_qdr_address_config_t(addr);
}
@@ -678,6 +684,11 @@ void qdr_core_remove_address_config(qdr_core_t *core, qdr_address_config_t *addr
qd_iterator_t *pattern = qd_iterator_string(addr->pattern, ITER_VIEW_ALL);
// Remove the address from the list and the parse tree
+ if (addr->hash_handle) {
+ qd_hash_remove_by_handle(core->addr_lr_al_hash, addr->hash_handle);
+ qd_hash_handle_free(addr->hash_handle);
+ addr->hash_handle = 0;
+ }
DEQ_REMOVE(core->addr_config, addr);
qd_parse_tree_remove_pattern(core->addr_parse_tree, pattern);
addr->ref_count--;
diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h
index 6b209d0..5f358d0 100644
--- a/src/router_core/router_core_private.h
+++ b/src/router_core/router_core_private.h
@@ -607,6 +607,7 @@ struct qdr_address_config_t {
int in_phase;
int out_phase;
int priority;
+ qd_hash_handle_t *hash_handle;
};
DEQ_DECLARE(qdr_address_config_t, qdr_address_config_list_t);
@@ -713,6 +714,7 @@ struct qdr_link_route_t {
char *add_prefix;
char *del_prefix;
qdr_connection_t *parent_conn;
+ qd_hash_handle_t *hash_handle;
};
void qdr_core_delete_link_route(qdr_core_t *core, qdr_link_route_t *lr);
@@ -760,6 +762,7 @@ struct qdr_auto_link_t {
qdr_core_timer_t *retry_timer; // If the auto link attach fails or gets disconnected, this timer retries the attach.
char *last_error;
bool fallback; // True iff this auto-link attaches to a fallback destination for an address.
+ qd_hash_handle_t *hash_handle;
};
DEQ_DECLARE(qdr_auto_link_t, qdr_auto_link_list_t);
@@ -847,6 +850,9 @@ struct qdr_core_t {
int worker_thread_count;
qdr_address_config_list_t addr_config;
+ // Hash to hold names of address configs, link routes and auto links.
+ // address config prefix = 'C', auto link prefix = 'A', link route prefix = 'L'
+ qd_hash_t *addr_lr_al_hash;
qdr_auto_link_list_t auto_links;
qdr_link_route_list_t link_routes;
qd_hash_t *conn_id_hash;
diff --git a/tests/system_tests_autolinks.py b/tests/system_tests_autolinks.py
index 434c578..19d1c0e 100644
--- a/tests/system_tests_autolinks.py
+++ b/tests/system_tests_autolinks.py
@@ -32,6 +32,7 @@ from proton.handlers import MessagingHandler
from proton.reactor import Container
from subprocess import PIPE, STDOUT
from qpid_dispatch.management.client import Node
+from system_test import QdManager
CONNECTION_PROPERTIES = {u'connection': u'properties', u'int_property': 6451}
@@ -92,6 +93,119 @@ class AutoLinkDetachAfterAttachTest(MessagingHandler):
def run(self):
Container(self).run()
+class NameCollisionTest(TestCase):
+ @classmethod
+ def setUpClass(cls):
+ super(NameCollisionTest, cls).setUpClass()
+ name = "test-router"
+
+ config = Qdrouterd.Config([
+
+ ('router', {'mode': 'standalone', 'id': 'A'}),
+ ('listener', {'host': '127.0.0.1', 'role': 'normal',
+ 'port': cls.tester.get_port()}),
+ ('autoLink', {'name': 'autoLink',
+ 'address': 'autoLink1',
+ 'connection': 'brokerConnection',
+ 'direction': 'in'}),
+ ('linkRoute', {'name': 'linkRoute',
+ 'prefix': 'linkRoute',
+ 'connection': 'brokerConnection',
+ 'direction': 'in'}),
+ ('address', {'name': 'address',
+ 'prefix': 'address.1',
+ 'waypoint': 'yes'}),
+ ])
+
+ cls.router = cls.tester.qdrouterd(name, config)
+ cls.router.wait_ready()
+ cls.address = cls.router.addresses[0]
+
+ def test_name_collision(self):
+ args = {"name": "autoLink", "address": "autoLink1", "connection": "broker", "dir": "in"}
+ # Add autoLink with the same name as the one already present.
+ al_long_type = 'org.apache.qpid.dispatch.router.config.autoLink'
+ addr_long_type = 'org.apache.qpid.dispatch.router.config.address'
+ lr_long_type = 'org.apache.qpid.dispatch.router.config.linkRoute'
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ test_pass = False
+ try:
+ mgmt.create(al_long_type, args)
+ except Exception as e:
+ if "BadRequestStatus: Name conflicts with an existing entity" in str(e):
+ test_pass = True
+ self.assertTrue(test_pass)
+
+ # Try to add duplicate linkRoute and make sure it fails
+ args = {"name": "linkRoute", "prefix": "linkRoute",
+ "connection": "broker", "dir": "in"}
+
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ test_pass = False
+ try:
+ mgmt.create(lr_long_type, args)
+ except Exception as e:
+ if "BadRequestStatus: Name conflicts with an existing entity" in str(e):
+ test_pass = True
+ self.assertTrue(test_pass)
+
+ args = {"name": "address", "prefix": "address.1",
+ "waypoint": "yes"}
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ test_pass = False
+ try:
+ mgmt.create(addr_long_type, args)
+ except Exception as e:
+ if "BadRequestStatus: Name conflicts with an existing entity" in str(e):
+ test_pass = True
+ self.assertTrue(test_pass)
+
+ # The linkRoutes, autoLinks and addrConfigs share the same hashtable
+ # but with a prefix.
+ # The following tests make sure that same names used on
+ # different entities are allowed.
+
+ # insert a linkRoute with the name of an existing autoLink and make
+ # sure that is ok
+ args = {"name": "autoLink", "prefix": "linkRoute",
+ "connection": "broker", "dir": "in"}
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ mgmt.create(lr_long_type, args)
+
+ # insert a linkRoute with the name of an existing addr config and make
+ # sure that is ok
+ args = {"name": "address", "prefix": "linkRoute",
+ "connection": "broker", "dir": "in"}
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ mgmt.create(lr_long_type, args)
+
+ # insert an autoLink with the name of an existing linkRoute and make
+ # sure that is ok
+ args = {"name": "linkRoute", "address": "autoLink1", "connection": "broker", "dir": "in"}
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ mgmt.create(al_long_type, args)
+
+ # insert an autoLink with the name of an existing address and make
+ # sure that is ok
+ args = {"name": "address", "address": "autoLink1", "connection": "broker", "dir": "in"}
+ al_long_type = 'org.apache.qpid.dispatch.router.config.autoLink'
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ mgmt.create(al_long_type, args)
+
+ # insert an address with the name of an existing autoLink and make
+ # sure that is ok
+ args = {"name": "autoLink", "prefix": "address.2",
+ "waypoint": "yes"}
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ mgmt.create(addr_long_type, args)
+
+ # insert an autoLink with the name of an existing linkRoute and make
+ # sure that is ok
+ args = {"name": "linkRoute", "prefix": "address.3",
+ "waypoint": "yes"}
+ mgmt = QdManager(self, address=self.router.addresses[0])
+ mgmt.create(addr_long_type, args)
+
class DetachAfterAttachTest(TestCase):
@classmethod
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org