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

Make auto-bounds calc non-default #955

Closed
3 of 5 tasks
nyurik opened this issue Oct 20, 2023 · 1 comment · Fixed by #958
Closed
3 of 5 tasks

Make auto-bounds calc non-default #955

nyurik opened this issue Oct 20, 2023 · 1 comment · Fixed by #958
Labels
config Relates to Martin configuration enhancement

Comments

@nyurik
Copy link
Member

nyurik commented Oct 20, 2023 •
edited
Loading

It seems the automatic bbox boundary computation is frequently causes slowdowns on startup, and confusion among novice users. I personally also experienced it. Let's make bounds computation disabled by default.

Changes

  • Add a new --bounds enum config option with values none (default) and auto. TBD: Should it be full, world, all or planet instead of none?
  • Remove --disable_bounds option, or at least make it deprecated for now, and prevent its usage at the same time as --bounds
  • Config should also support the same values, e.g. bounds: auto in addition to bounds: [-180,-90,180,90]
  • Config should obsolete the disabled_bounds and instead add a bounds value to the auto_publish.tables section - from PgConfig to PgCfgPublishType
    • I think we should also move default_srid, max_feature_count to the auto_publish.tables section.

To consider

It might be good to do asynchronous bounds computation, where it gets done after Martin startup, and initially Martin uses full bounds until the computation is done... This might be a lot trickier though, because we will need to modify how all sources are stored and updated. At the moment, all sources are immutable and exist for the entire duration of the program, but this may change as part of the source auto-refresh work (ability to re-query the database to add new sources on the fly #288)

See also:
@nyurik nyurik added enhancement help wanted config Relates to Martin configuration labels Oct 20, 2023
@nyurik nyurik mentioned this issue Oct 21, 2023
Merged
@nyurik
Copy link
Member Author

nyurik commented Oct 22, 2023

See implementation ^, some changes from the initial proposal, and on the second thought I don't think moving it to auto-publish makes sense

nyurik added a commit that referenced this issue Oct 22, 2023
@nyurik
[BREAKING] New bounds calculation methods (#958)
7aa169e 
* Remove `--disable-bounds` flag and `disable_bounds` config parameters.
* Add `--auto-bounds` / `-b` CLI parameter and `auto_bounds` config
value:
* `quick`: Compute table geometry bounds, but abort if it takes longer
than 5 seconds (default)
* `calc`: Compute table geometry bounds. The startup time may be
significant. Make sure all GEO columns have indexes
* `skip`: Skip bounds calculation. The bounds will be set to the whole
world
* `-b` is now mapped to `--auto-bounds` param, but it will fail if used
by itself because it now requires a value.

Fixes #955
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
config Relates to Martin configuration enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[BREAKING] New bounds calculation methods nyurik/martin
1 participant
@nyurik