You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by mgoulish <gi...@git.apache.org> on 2018/09/26 17:32:17 UTC
[GitHub] qpid-dispatch pull request #384: DISPATCH-1096 - priority sheaf cleanup bug
GitHub user mgoulish opened a pull request:
https://github.com/apache/qpid-dispatch/pull/384
DISPATCH-1096 - priority sheaf cleanup bug
This is for the bug that Ganesh and Chuck saw while killing and restarting routers.
The test succeeded 10 times each (with 0 false results) in these 4 cases:
{ Debug, RelWithDebInfo } x { Should_Fail, Should_Succeed }
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mgoulish/qpid-dispatch priority_cleanup_bug
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/qpid-dispatch/pull/384.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #384
----
commit f25b80641bf05e375626e608666823678ba8f078
Author: Michael Goulish <mg...@...>
Date: 2018-09-26T17:26:59Z
DISPATCH-1096 - priority sheaf cleanup bug
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch issue #384: DISPATCH-1096 - priority sheaf cleanup bug
Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on the issue:
https://github.com/apache/qpid-dispatch/pull/384
> This patch clears up the too-many-links for sheaf error and improves my router network stability. FOr this I approve the patch.
>
> But in tests before and after this patch my throughput went from 2600 msg/S to 200-300 msg/S. The performance drop needs to be addressed as a separate issue.
I don't see any changes in this PR that should affect throughput performance.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch issue #384: DISPATCH-1096 - priority sheaf cleanup bug
Posted by mgoulish <gi...@git.apache.org>.
Github user mgoulish commented on the issue:
https://github.com/apache/qpid-dispatch/pull/384
I yanked that exit(1).
I thought I needed that to detect failure in my python test, but that is not true.
Tests all still did the right thing, in 10 trials each (40 total) in the set: {should_fail, should_succeed} * { DEBUG, RelWithDebInfo}
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch pull request #384: DISPATCH-1096 - priority sheaf cleanup bug
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/qpid-dispatch/pull/384
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch pull request #384: DISPATCH-1096 - priority sheaf cleanup bug
Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on a diff in the pull request:
https://github.com/apache/qpid-dispatch/pull/384#discussion_r220714152
--- Diff: src/router_core/connections.c ---
@@ -1466,8 +1470,8 @@ static void qdr_attach_link_data_CT(qdr_core_t *core, qdr_connection_t *conn, qd
// are assigned priorities in the order in which they are attached.
int next_slot = core->data_links_by_mask_bit[conn->mask_bit].count ++;
if (next_slot >= QDR_N_PRIORITIES) {
- qd_log(core->log, QD_LOG_ERROR, "Attempt to attach too many inter-router links for priority sheaf.");
- return;
+ qd_log(core->log, QD_LOG_CRITICAL, "Attempt to attach too many inter-router links for priority sheaf.");
+ exit(1);
--- End diff --
Exiting the process here is a bit heavy handed. I think it's sufficient to log the critical error.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch issue #384: DISPATCH-1096 - priority sheaf cleanup bug
Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on the issue:
https://github.com/apache/qpid-dispatch/pull/384
> > This patch clears up the too-many-links for sheaf error and improves my router network stability. FOr this I approve the patch.
> > But in tests before and after this patch my throughput went from 2600 msg/S to 200-300 msg/S. The performance drop needs to be addressed as a separate issue.
>
> I don't see any changes in this PR that should affect throughput performance.
I withdraw the above statement. There appears to be a memory leak that is causing performance (and other) problems.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch issue #384: DISPATCH-1096 - priority sheaf cleanup bug
Posted by ChugR <gi...@git.apache.org>.
Github user ChugR commented on the issue:
https://github.com/apache/qpid-dispatch/pull/384
This patch clears up the too-many-links for sheaf error and improves my router network stability. FOr this I approve the patch.
But in tests before and after this patch my throughput went from 2600 msg/S to 200-300 msg/S. The performance drop needs to be addressed as a separate issue.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch issue #384: DISPATCH-1096 - priority sheaf cleanup bug
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/qpid-dispatch/pull/384
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/384?src=pr&el=h1) Report
> Merging [#384](https://codecov.io/gh/apache/qpid-dispatch/pull/384?src=pr&el=desc) into [master](https://codecov.io/gh/apache/qpid-dispatch/commit/fa03ea4008524ca7eb5ba4c6409ec0328eca2e87?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/384?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #384 +/- ##
==========================================
- Coverage 84.8% 84.79% -0.01%
==========================================
Files 73 73
Lines 16462 16467 +5
==========================================
+ Hits 13960 13964 +4
- Misses 2502 2503 +1
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/384?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/384/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=) | `92.18% <100%> (-0.23%)` | :arrow_down: |
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/384/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `93.08% <50%> (-0.21%)` | :arrow_down: |
| [src/router\_core/agent\_link.c](https://codecov.io/gh/apache/qpid-dispatch/pull/384/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50X2xpbmsuYw==) | `63.21% <0%> (-0.58%)` | :arrow_down: |
| [src/router\_core/core\_test\_hooks.c](https://codecov.io/gh/apache/qpid-dispatch/pull/384/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2NvcmVfdGVzdF9ob29rcy5j) | `88.06% <0%> (+1.7%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/384?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/384?src=pr&el=footer). Last update [fa03ea4...f25b806](https://codecov.io/gh/apache/qpid-dispatch/pull/384?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org