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.

Sign up for GitHub

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

Jump to bottom

reactor: pass fd opened in blocking mode to spawned process #1542

Closed
wants to merge 1 commit into from
Closed

reactor: pass fd opened in blocking mode to spawned process #1542

wants to merge 1 commit into from

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Mar 13, 2023 •
edited
Loading

the fix does not address this issue. i will close it after @balusch acks it.

it turns out tools like "cat" expect fd opened blocking mode. while the pipe fds are always created in non-blocking mode. so in order to appease these tools, let's set the fds passed to the spawned process to blocking mode.

Fixes #1320
Signed-off-by: Jianyong Chen baluschch@gmail.com
Signed-off-by: Kefu Chai kefu.chai@scylladb.com

@balusch
Copy link
Contributor

balusch commented Mar 13, 2023

LGTM. But should we also remove the decorator in test_spawn_input?

@tchaikov
reactor: pass fd opened in blocking mode to spawned process
486f38d 
* pass fds opened in blocking mode to spawned process
* do not tolerate test failures in test_spawn_input

it turns out tools like "cat" expect fd opened blocking mode. while
the pipe fds are always created in non-blocking mode. so in order to
appease these tools, let's set the fds passed to the spawned process
to blocking mode.

Fixes scylladb#1320
Signed-off-by: Jianyong Chen <baluschch@gmail.com>
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
@tchaikov tchaikov force-pushed the reactor-blocking-fd-for-spawned-process branch from 133e1b5 to 486f38d Compare March 13, 2023 14:39
@tchaikov
Copy link
Contributor Author

LGTM. But should we also remove the decorator in test_spawn_input?

yes. fixed and repushed.

@balusch
Copy link
Contributor

balusch commented Mar 13, 2023

CI passed! 💪

@tchaikov
Copy link
Contributor Author

tchaikov commented Mar 14, 2023 •
edited
Loading

i am still running the previously failing test. will report back once once the counter reaches 65535.

@tchaikov
Copy link
Contributor Author 

===========1158==========
Running 6 test cases...
WARNING: debug mode. Not for benchmarking or production
INFO  2023-03-14 21:14:12,781 seastar - Reactor backend: io_uring
INFO  2023-03-14 21:14:13,599 [shard 20] seastar - Created fair group io-queue-0, capacity rate 2147483:2147483, limit 12582912, rate 16777216 (factor 1), threshold 2000
INFO  2023-03-14 21:14:14,056 [shard 20] seastar - IO queue uses 0.75ms latency goal for device 0
INFO  2023-03-14 21:14:14,106 [shard 20] seastar - Created io group dev(0), length limit 4194304:4194304, rate 2147483647:2147483647
INFO  2023-03-14 21:14:14,225 [shard  7] seastar - Created fair group io-queue-0, capacity rate 2147483:2147483, limit 12582912, rate 16777216 (factor 1), threshold 2000
INFO  2023-03-14 21:14:14,226 [shard  7] seastar - IO queue uses 0.75ms latency goal for device 0
INFO  2023-03-14 21:14:14,291 [shard  7] seastar - Created io group dev(0), length limit 4194304:4194304, rate 2147483647:2147483647
INFO  2023-03-14 21:14:14,292 [shard  0] seastar - Created io queue dev(0) capacities: 512:2000:2000 1024:3000:3000 2048:5000:5000 4096:9000:9000 8192:17000:17000 16384:33000:33000 32768:65000:65000 65536:129000:129000 131072:257000:257000
random-seed=1302983379
/home/kefu/dev/seastar/tests/unit/spawn_test.cc(104): error: in "test_spawn_input": failed to write to stdin: std::system_error (error system:32, Broken pipe)
/home/kefu/dev/seastar/tests/unit/spawn_test.cc(111): error: in "test_spawn_input": check sstring(echo.get(), echo.size()) == text has failed [ != hello world
]

@tchaikov tchaikov marked this pull request as draft March 14, 2023 13:21
@balusch
Copy link
Contributor

balusch commented Mar 14, 2023

So sad. 😢

I'm home now an no Linux machine around. You can close this PR first and I'll make more invetigations when I'm free.

@tchaikov
Copy link
Contributor Author

yup. It's sad. Thank you for your help!

@tchaikov tchaikov closed this Mar 14, 2023
@tchaikov tchaikov deleted the reactor-blocking-fd-for-spawned-process branch March 14, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test_spawn_input fails randomly
2 participants
@tchaikov @balusch