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

Check if ports are occupied before start sqlness test #1602

Closed
waynexia opened this issue May 17, 2023 · 11 comments
Closed

Check if ports are occupied before start sqlness test #1602

waynexia opened this issue May 17, 2023 · 11 comments
Assignees
Labels
C-feature Category Features good first issue Good for newcomers

Comments

@waynexia
Copy link
Member

What problem does the new feature solve?

Sqlness uses the common port (e.g. 4000) to start server. It's common that the port is occupied by other process like another greptimedb process. In this case the test result will become weird because it's actually testing something else

What does the feature do?

Check those ports before spawn subprocess. Sqlness only check if the port is up after spawn at present.

Implementation challenges

No response

@waynexia waynexia added good first issue Good for newcomers C-feature Category Features Size: S labels May 17, 2023
@killme2008 killme2008 self-assigned this Jun 2, 2023
@Gump9
Copy link
Contributor

Gump9 commented Jul 3, 2023

I'm a little confused about

Check those ports before spawn subprocess.

it that means Sqlness will spawn subprocess?

Which file can give me a first look at the problem?

Sqlness only check if the port is up after spawn at present.

@Gump9
Copy link
Contributor

Gump9 commented Jul 3, 2023

Sry,i don't see it has been assigned😭

@WenyXu
Copy link
Member

WenyXu commented Jul 3, 2023

Sry,i don't see it has been assigned😭

Never mind. We got a ton of the good first issues waiting for you 🤪

@evenyag
Copy link
Contributor

evenyag commented Jul 3, 2023

@Gump9 Are you still interested in this issue? Feel free to take it.

@Gump9
Copy link
Contributor

Gump9 commented Jul 3, 2023

@Gump9 Are you still interested in this issue? Feel free to take it.

Yes,can I have a try?tks alot

@Gump9
Copy link
Contributor

Gump9 commented Jul 3, 2023

and i still want to know more details about this issue i mentioned above

@waynexia
Copy link
Member Author

waynexia commented Jul 4, 2023

it that means Sqlness will spawn subprocess?

Which file can give me a first look at the problem?

Yes, sqlness runner will start greptimedb (metasrv/frontend/datanode) to test. You can check it out here

async fn start_server(

@Gump9
Copy link
Contributor

Gump9 commented Jul 4, 2023

@waynexia thx a lot😄

@Gump9
Copy link
Contributor

Gump9 commented Jul 5, 2023

@waynexia Hi!should i just panic like the code below?because i found "Sqlness only check if the port is up after spawn at present." do something like this:

if !util::check_port(check_ip_addr.parse().unwrap(), Duration::from_secs(10)).await {
            Env::stop_server(&mut process);
            panic!("{subcommand} doesn't up in 10 seconds, quit.")
}

or,should i choose another port and try to up a process again?

@waynexia
Copy link
Member Author

waynexia commented Jul 5, 2023

panic should be enough. We can add one config entry for port later

@Gump9
Copy link
Contributor

Gump9 commented Jul 5, 2023

panic should be enough. We can add one config entry for port later

thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category Features good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants