Skip to content

Use manifest to skip COPY of empty partitions when writing to Redshift - #99

Closed
JoshRosen wants to merge 8 commits into
masterfrom
fix-empty-avro-partitions
Closed

Use manifest to skip COPY of empty partitions when writing to Redshift#99
JoshRosen wants to merge 8 commits into
masterfrom
fix-empty-avro-partitions

Conversation

@JoshRosen

@JoshRosen JoshRosen commented Sep 29, 2015

Copy link
Copy Markdown
Contributor

This WIP patch fixes #96, an issue where Redshift's COPY command did not cope well with empty Avro partitions.

The fix implemented here is to use a manifest to instruct Redshift to load only the non-empty partitions' Avro files.

@JoshRosen JoshRosen added the bug label Sep 29, 2015
@JoshRosen JoshRosen added this to the 0.5.1 milestone Sep 29, 2015
@codecov-io

codecov-io commented Sep 29, 2015

Copy link
Copy Markdown

Current coverage is 95.10%

Merging #99 into master will increase coverage by +0.29% as of 216aabb

@@            master    #99   diff @@
=====================================
  Files           11     11       
  Stmts          444    470    +26
  Branches       109    115     +6
  Methods          0      0       
=====================================
+ Hit            421    447    +26
  Partial          0      0       
  Missed          23     23

Review entire Coverage Diff as of 216aabb

Powered by Codecov. Updated on successful CI builds.

@JoshRosen JoshRosen changed the title [WIP] Use manifest to skip copy of empty partitions when writing to Redshift [WIP] Use manifest to skip COPY of empty partitions when writing to Redshift Sep 30, 2015
@JoshRosen JoshRosen changed the title [WIP] Use manifest to skip COPY of empty partitions when writing to Redshift Use manifest to skip COPY of empty partitions when writing to Redshift Sep 30, 2015
@JoshRosen

JoshRosen commented Sep 30, 2015

Copy link
Copy Markdown
Contributor Author

Alright, this should now be ready for review. /cc @marmbrus and @cfeduke for comments.

JoshRosen
JoshRosen reviewed Sep 30, 2015
View reviewed changes
Comment thread src/main/scala/com/databricks/spark/redshift/RedshiftWriter.scala

@JoshRosen JoshRosen Sep 30, 2015

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.

The only change in this block was to wrap the actual COPY statement itself in this foreach block in order to skip the copy if no non-empty Avro partitions were written.

@cfeduke

cfeduke commented Sep 30, 2015

Copy link
Copy Markdown

I will run this patch with the same queries that were causing issues tonight; specifically I know of a query that actually causes 0 rows to be returned (something from last Thanksgiving) which would be a decent full coverage test.

@JoshRosen

JoshRosen commented Oct 2, 2015

Copy link
Copy Markdown
Contributor Author

@cfeduke, did you end up getting a chance to try this out? I'm fairly confident that this should fix the issue, given the regression tests that I added, but more evidence from a real use-case never hurts.

@marmbrus

marmbrus commented Oct 5, 2015

Copy link
Copy Markdown
Contributor

LGTM

@JoshRosen

JoshRosen commented Oct 5, 2015

Copy link
Copy Markdown
Contributor Author

Alright, going to merge this now (given that it has integration tests).

@JoshRosen JoshRosen closed this in 2a1a4d5 Oct 5, 2015
@JoshRosen
JoshRosen deleted the fix-empty-avro-partitions branch October 5, 2015 22:01
This was referenced Oct 24, 2015
Closed
Closed
JoshRosen added a commit that referenced this pull request Oct 31, 2015
@JoshRosen
Restore compatibility with spark-avro 1.0.0
1293621 
This patch addresses an issue where `spark-redshift` would run into "Mandatory url is not present in manifest file" errors when saving data back to Redshift if a pre-2.0.0 version of `spark-avro` was used at runtime. This problem should not arise for most users of `spark-redshift`, since the proper version of `spark-avro` should automatically be pulled in via Maven or Ivy; the goal of this patch is to provide compatibility for users who cannot upgrade `spark-avro` for other compatibility-related reasons.

**Cause of the write path bug**: In order to fix a crash that could occur when writing tables containing empty partitions, #99 modified the write path to use manifest files that instruct Redshift to load only the non-empty partitions' Avro files. The code that handled processing of filenames when generating the manifest made assumptions about filenames that hold for spark-avro 2.0.0+ but not for spark-avro 1.0.0.

**Solution**: use a different method to build the correct list of part file names when constructing the manifest.

Fixes #111.

Author: Josh Rosen <joshrosen@databricks.com>

Closes #114 from JoshRosen/compatibility-with-spark-avro-1.0.
@JoshRosen JoshRosen mentioned this pull request Dec 11, 2015
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

0.5.1

Development

Successfully merging this pull request may close these issues.

Avro schema only file causes Redshift stl_load_errors: "Invalid AVRO file found. Unexpected end of AVRO file."

4 participants

@JoshRosen @codecov-io @cfeduke @marmbrus