Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Limit the number of extracts possible with osmium extract
So we don't run out of file descriptors.

See #265.
  • Loading branch information
joto committed May 2, 2023
1 parent 513e0de commit 353e9d5
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
6 changes: 6 additions & 0 deletions man/osmium-extract.md
Expand Up @@ -391,6 +391,12 @@ steps. First create several larger extracts and then split them again and
again into smaller pieces.


# LIMITS

You can not have more than 500 extracts. Although chances are that you will be
running out of memory long before that. See MEMORY USAGE.


# EXAMPLES

See the example config files in the *extract-example-config* directory. To
Expand Down
6 changes: 6 additions & 0 deletions src/command_extract.cpp
Expand Up @@ -72,6 +72,8 @@ along with this program. If not, see <https://www.gnu.org/licenses/>.
# include <unistd.h>
#endif

static constexpr const std::size_t max_extracts = 500;

This comment has been minimized.

Copy link
@frafra

frafra May 18, 2023

This prevents users on systems that can open more than 500 file descriptors per process to use osmium as intended, as well as not handling the issue for users on systems with lower numbers. This would just work with environments where the limit is exactly 500.

This comment has been minimized.

Copy link
@joto

joto May 19, 2023

Author Member

This works for 99.99% of users giving them some indication that what they are doing is problematic. I'd rather have an all singing all dancing solution that's perfect and never fails and all that, but you complained about Osmium not handling this case and now that it does in some way it is also wrong. Sorry, can't have it both ways.

This comment has been minimized.

Copy link
@frafra

frafra May 19, 2023

I agree that having a patch that improves the situation is better than not having a patch. I am arguing that the software would be better without this patch, actually. The error message was already shown, and it hinted the problem. This patch hides the real issue and limits artificially the ability of the software.
It would be enough to list as limitation mentioning ulimit. It is just my opinion, of course.


static osmium::Box parse_bbox(const rapidjson::Value& value) {
if (value.IsArray()) {
if (value.Size() != 4) {
Expand Down Expand Up @@ -622,6 +624,10 @@ bool CommandExtract::run() {
throw config_error{"No extract specified in config file or on the command line."};
}

if (m_extracts.size() > max_extracts) {
throw config_error{"Too many extracts specified in config file (Maximum: " + std::to_string(max_extracts) + ")."};
}

show_extracts();

m_strategy = make_strategy(m_strategy_name);
Expand Down

0 comments on commit 353e9d5

Please sign in to comment.