Conversation
When several gateway plugins publish an entity with the same ID, such as a parent area each of them creates by default, the cache refresh registers them in load order and every registration takes the ID over. The ID therefore passes from plugin to plugin on every refresh, and each transfer logged a WARN line: with four such plugins that was four lines per refresh, for as long as the gateway ran. Ownership is unchanged. The plugin loaded last among the publishers owns the ID at the end of every refresh. Outside hybrid discovery this is also the plugin whose copy of the entity the cache serves, and when the owner stops publishing the ID another plugin that still publishes it owns it by the end of the same refresh. Only the log changes. A transfer between the same two plugins over the same ID is logged once, not on every refresh. The logged pairs are kept while the ID has an owner. PluginManager::finish_ownership_refresh(), called once at the end of every refresh, drops the pairs of IDs that no plugin owns any more, so the record stays bounded by the owned IDs and a conflict that comes back later is logged again. The plugin system tutorial describes the rule and the warning.
| // log each pair of them once. | ||
| const auto & [first, second] = std::minmax(it->second, plugin_name); | ||
| if (reported_ownership_conflicts_.emplace(eid, first, second).second) { | ||
| RCLCPP_WARN(logger(), |
There was a problem hiding this comment.
When an id already has an owner from the previous refresh, the first transfer seen is "previous owner -> first publisher in load order", the reverse of where the id ends up, and the sorted pair then suppresses the real direction for good: with plugin_b owning shared_area and plugin_a starting to publish it, the only line ever logged is "transferred from plugin_b to plugin_a" while plugin_b keeps the id and its copy is served. With four publishers the same mechanism adds a "plugin_d to plugin_a" line one refresh after the first three. Log where the outcome is known instead: collect each id's publishers during the refresh and let finish_ownership_refresh() emit one line per id naming the publishers and the final owner, deduplicated on the id plus its publisher set.
|
|
||
| // Pairs in the order ownership moves: a-b, b-c, c-d in the first refresh, | ||
| // d-a in the second. | ||
| ASSERT_EQ(first_two.size(), 4u); |
There was a problem hiding this comment.
This pins the fourth line, "plugin_d to plugin_a", which is the previous refresh's leftover owner colliding with the first publisher and is undone within the same refresh, not a hand-over an operator should be told about. Once the warning moves to the end of the refresh this becomes one line for shared_area naming plugin_d as owner, and the test should assert that.
| if (capture == nullptr) { | ||
| return; | ||
| } | ||
| std::lock_guard<std::mutex> lk(capture->mutex_); |
There was a problem hiding this comment.
Between active().load() and this lock nothing stops ~LogCapture() on the test thread from restoring the handler and destroying the object, so a WARN from another thread (the REST server thread or an executor callback in the notify test) locks a freed mutex. Guard the pointer and the push with one static mutex and take that same mutex in the destructor around active().store(nullptr), which also makes mutex_ redundant.
| - When several plugins publish the same entity ID, for example a parent area that each of | ||
| them creates by default, the plugin loaded last among them owns it. Each refresh registers | ||
| the plugins in load order and every registration takes the ID over, so during a refresh | ||
| the ID passes from plugin to plugin and ends with the last one. Outside hybrid discovery |
There was a problem hiding this comment.
"Outside hybrid discovery" hides that hybrid does the opposite: PluginLayer defaults to MergePolicy::ENRICHMENT, so the merge pipeline keeps the field values of the plugin loaded first and only fills gaps from later plugins, while routing goes to the plugin loaded last. State that here, since this paragraph justifies last-wins by the served copy.
This branch has not been deployed
2 participants
Summary
When several plugins publish an entity with the same id (for example an area that more than one plugin uses by
default), the plugin that registers last in a refresh owns it, which is also the plugin whose copy the entity cache
serves. That rule is right, but every refresh hands the id from plugin to plugin in load order and each hand-over
logged a warning. A gateway with four such plugins logged four ownership warnings on every refresh, indefinitely.
Ownership is unchanged. A transfer between the same two plugins over the same id is now logged once, and forgotten
when a refresh ends with the id owned by no plugin, so a conflict that returns later is reported again.
Issue
Type
What changes
PluginManager::register_entity_ownershipkeeps last-wins ownership and remembers which pairs of plugins italready reported for an id.
PluginManager::finish_ownership_refresh(), called once at the end of each entity refresh, drops the rememberedpairs of ids nobody owns any more, so the record stays bounded by the conflicts over ids that are currently owned.
refresh, and when the warning is logged.
Testing
once, four publishers log each pair once, an owner that stops publishing hands the id to the remaining publisher
in the same refresh in both load orders, the owner and the served copy come from the same plugin, a forgotten
conflict is reported again, a single plugin unchanged. Two tests drive a real gateway node's refresh. Each was
shown to fail on the previous code or under an injection.
changed files, and a docs build with no new warning.
Checklist