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

[GAIAPLAT-1236] db_server - better error message when 'Address already in use' #881

Merged
merged 5 commits into from
Aug 28, 2021
Merged

[GAIAPLAT-1236] db_server - better error message when 'Address already in use' #881

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ca030ff
[GAIAPLAT-1236] db_server - better error message when 'Address alread…
simone-gaia Aug 27, 2021
5be6309
Fix error message:
simone-gaia Aug 27, 2021
d0dffa0
Revert formatting
simone-gaia Aug 27, 2021
081c5fc
Imporve messages
simone-gaia Aug 27, 2021
c7c03c1
Address PR feedback
simone-gaia Aug 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions production/db/core/src/db_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,14 @@ void server_t::init_listening_socket(const std::string& socket_name)
socklen_t server_addr_size = sizeof(server_addr.sun_family) + 1 + ::strlen(&server_addr.sun_path[1]);
if (-1 == ::bind(listening_socket, reinterpret_cast<struct sockaddr*>(&server_addr), server_addr_size))
{
// TODO it would be nice to have a common error handler that can handle common errors.
if (errno == EADDRINUSE)
{
cerr << "ERROR: bind() failed! - " << (::strerror(errno)) << endl;
Copy link

Choose a reason for hiding this comment

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

Is this for our use? If not what useful information does it provide to the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, more skilled users will know what the syscall bind is. For now, we just have this one but in the future, we may have more, and knowing exactly what went wrong can be helpful. This can also be used by the user to report the errors back to us.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the pattern should be that we print the friendly error message before the detailed message/stack trace. (You could also avoid duplicating the system error message by putting this in a try/catch block and printing e.what(), but maybe it's simpler this way for now.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed the same pattern and formatting used in gaiac. At the moment I'm trying to make things consistent. Once we re-design the CLI we can re-evaluate these decisions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

➜  cmake-build-gaiarelease-debug git:(wayne/test-auto-conn-discon) ✗ catalog/gaiac/gaiac ../schemas/test/one_to_one/one_to_one.ddl                                      
ERROR: Connect failed! - Connection refused
Can't connect to a running instance of the Gaia Database Server.
Start the Gaia Database Server and rerun gaiac.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, that's not so bad (as long as there's no stack trace before the friendly message). Approved.

cerr << "The Gaia Database Server cannot start because another instance is already running. Please stop any instances of the server that are running." << endl;
Copy link

Choose a reason for hiding this comment

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

could there be non running instances of the server that could cause an issue?

Maybe
Stop any instances of the server and try again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could there be non running instances of the server that could cause an issue?

Potentially you could create an address with the same name, but very unlikely.

Stop any instances of the server and try again.

Changed.

exit(1);
LaurentiuCristofor marked this conversation as resolved.
Show resolved Hide resolved
}

throw_system_error("bind() failed!");
LaurentiuCristofor marked this conversation as resolved.
Show resolved Hide resolved
}
if (-1 == ::listen(listening_socket, 0))
Expand Down Expand Up @@ -2596,3 +2604,7 @@ void server_t::run(server_config_t server_conf)
}
}
}

void server_t::handle_common_errors()
{
}
4 changes: 3 additions & 1 deletion production/db/core/src/db_server_exec.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ static server_config_t process_command_line(int argc, char* argv[])
}
}

std::cerr << "Starting " << c_db_server_name << "..." << std::endl;

gaia_config_fallback_t gaia_conf{conf_file_path};

data_dir = gaia_conf.get_value<std::string>(
Expand Down Expand Up @@ -374,7 +376,7 @@ int main(int argc, char* argv[])
{
auto server_conf = process_command_line(argc, argv);

std::cerr << "Starting " << c_db_server_exec_name << "..." << std::endl;
std::cerr << c_db_server_name << " started!" << std::endl;

gaia::db::server_t::run(server_conf);
}