Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use non-default ports for clickhouse #13

Merged
merged 5 commits into from
Jan 28, 2025
Merged

use non-default ports for clickhouse #13

merged 5 commits into from
Jan 28, 2025

Conversation

olegrok
Copy link
Contributor

@olegrok olegrok commented Jan 27, 2025

Two tests (cometa + journald_forwarder) conflicted because of Clickhouse. This patch fixes it.

@olegrok olegrok force-pushed the fix-other-par-workflow branch from 9fd491e to b612bb6 Compare January 27, 2025 16:19
@olegrok olegrok force-pushed the fix-other-par-workflow branch from b612bb6 to af37cfa Compare January 27, 2025 16:23
@olegrok olegrok force-pushed the fix-other-par-workflow branch from af37cfa to ae1d946 Compare January 27, 2025 17:37
@olegrok olegrok marked this pull request as ready for review January 27, 2025 17:39
@olegrok olegrok requested a review from avm January 27, 2025 17:40
@olegrok olegrok force-pushed the fix-other-par-workflow branch from ae1d946 to 1f9a530 Compare January 27, 2025 18:09
@olegrok olegrok force-pushed the fix-other-par-workflow branch from 13501c3 to 5b3c0ff Compare January 27, 2025 20:03
Copy link
Collaborator

@avm avm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the approach of assigning ports to make tests parallel! I suggest we make a text file listing all of those assignments (like "nil/tests/cometa: 9002; nil/tests/journald_forwarder: 9001"), so that if you need a port for a new test, you don't have to search through all the tests.

@olegrok
Copy link
Contributor Author

olegrok commented Jan 27, 2025

Even with fixes I get

2025-01-27T20:18:44.9644193Z nil> ThreadSanitizer: failed to restore address 0x100675c0eb0
2025-01-27T20:18:44.9644532Z nil> FAIL       github.com/NilFoundation/nil/nil/services/synccommittee/prover/tracer   70.749s
2025-01-27T20:18:44.9644974Z nil> ok         github.com/NilFoundation/nil/nil/services/synccommittee/prover/tracer/internal/mpttracer        1.228s

I don't understand how we can fix ThreadSanitizer...

@olegrok
Copy link
Contributor Author

olegrok commented Jan 27, 2025

I like the approach of assigning ports to make tests parallel!

Ok. Done c50ae4d

Seems it helps to prevent following issue:
```
2025-01-27T20:18:44.9644193Z nil> ThreadSanitizer: failed to restore address 0x100675c0eb0
2025-01-27T20:18:44.9644532Z nil> FAIL       github.com/NilFoundation/nil/nil/services/synccommittee/prover/tracer   70.749s
2025-01-27T20:18:44.9644974Z nil> ok         github.com/NilFoundation/nil/nil/services/synccommittee/prover/tracer/internal/mpttracer        1.228s
```

I don't understand that is a reason of such failure but seems it helps
in case we decrease memory pressure during the test.
@avm avm added this pull request to the merge queue Jan 28, 2025
Merged via the queue into main with commit 9bed3df Jan 28, 2025
13 checks passed
@avm avm deleted the fix-other-par-workflow branch January 28, 2025 07:27
Gezort pushed a commit that referenced this pull request Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants