Skip to content

Repartition in ETL when re-tiling increases layer resolution - #2135

Merged
lossyrob merged 8 commits into
locationtech:masterfrom
echeipesh:fix/upsampling-etl-retile
Apr 20, 2017
Merged

lossyrob merged 8 commits into
locationtech:masterfrom
echeipesh:fix/upsampling-etl-retile

Conversation

@echeipesh

@echeipesh echeipesh commented Apr 11, 2017 •

Copy link
Copy Markdown
Contributor

During ETL process there are two ways in which the job specification can cause increase in resolution:

  • When maxZoom parameter forces a higher than necessary level using ZoomedLayoutScheme
  • When LayoutDefinition is provided that has higher resolution than source imagery

This is possible both in per-tile reprojected and buffered reproject methods.

If the increase in resolution is significant it will dramatically increase the size of the partitions, which were originally mapped to source imagery. Past certain point the partitions become too large to be processed.

For per-tile reproject this PR handles the logic in geotrellis.spark.etl.Etl class by inspecting the difference in resolutions of pre-tiled metadata and post-tiled metadata.

However in buffered reproject the check must happen during after the reprojection took place. The resolutions in different CRS are not comparable so we must check the tiles covered by the KeyBounds, assuming that the tiles themselves remain mostly constant in size.

Incidental Fixes

  • Render module was not callable due to incorrect name specification
  • Logging added to CutTiles for when a single resample may OOM
  • Etl must return maxZoom as zoom level when its specified

Resolves: #2129

@echeipesh

Copy link
Copy Markdown
Contributor Author

Buffered reproject ingest

buffered-ingest

@echeipesh
echeipesh force-pushed the fix/upsampling-etl-retile branch from c07af03 to 63a68c4 Compare April 11, 2017 04:48
import scala.reflect.ClassTag

object CutTiles {
@transient private lazy val logger = LazyLogging(this)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why you don't want to extend LazyLogging? API compatibility? Looks ugly and like we can forget about it ):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that was the intent is to avoid adding types to classes when all we want is this private field. I think this should be the prefered approach.

@@ -29,3 +29,9 @@ trait LazyLogging {
@transient protected lazy val logger: Logger =
Logger(LoggerFactory.getLogger(getClass.getName))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It you are sure in this approach, LazyLogging(this)

@lossyrob lossyrob added this to the 1.1 milestone Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants