Skip to content

[BEAM-2411] Make the write transform of HBaseIO simpler#3391

Merged
asfgit merged 2 commits intoapache:masterfrom
iemejia:BEAM-2411-make-hbaseio-write-simpler
Jun 20, 2017
Merged

[BEAM-2411] Make the write transform of HBaseIO simpler#3391
asfgit merged 2 commits intoapache:masterfrom
iemejia:BEAM-2411-make-hbaseio-write-simpler

Conversation

@iemejia
Copy link
Member

@iemejia iemejia commented Jun 19, 2017

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify.
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@iemejia
Copy link
Member Author

iemejia commented Jun 19, 2017

R: @jkff A simplification based on our discussion on the Bigtable-HBase duality.

@iemejia iemejia force-pushed the BEAM-2411-make-hbaseio-write-simpler branch 2 times, most recently from a82f283 to eee4350 Compare June 19, 2017 14:28
@iemejia
Copy link
Member Author

iemejia commented Jun 19, 2017

There is an extra thing, notice that I made elementCounters for debug per bundle, this is a fix on the previous version, and aligns it with BigtableIO.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.695% when pulling eee4350 on iemejia:BEAM-2411-make-hbaseio-write-simpler into 0cabdf6 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 70.691% when pulling eee4350 on iemejia:BEAM-2411-make-hbaseio-write-simpler into 0cabdf6 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.695% when pulling eee4350 on iemejia:BEAM-2411-make-hbaseio-write-simpler into 0cabdf6 on apache:master.

Copy link
Contributor

@jkff jkff left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM but also feel free to implement the suggested coder improvement.


public static final Coder<KV<byte[], Iterable<Mutation>>> WRITE_CODER =
KvCoder.of(ByteArrayCoder.of(), IterableCoder.of(HBaseMutationCoder.of()));
public static final Coder<Mutation> WRITE_CODER = HBaseMutationCoder.of();
Copy link
Contributor

Choose a reason for hiding this comment

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

While we're at simplifying things.. I believe you can remove this and all the setCoder calls by using something like https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryCoderProviderRegistrar.java

@iemejia
Copy link
Member Author

iemejia commented Jun 19, 2017

Thanks for the hint on the CoderProviderRegistrar, I am going to take a look and add it to the PR

@lukecwik
Copy link
Member

Retest this please

@iemejia iemejia force-pushed the BEAM-2411-make-hbaseio-write-simpler branch from eee4350 to d42f633 Compare June 20, 2017 08:01
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 70.684% when pulling d42f633 on iemejia:BEAM-2411-make-hbaseio-write-simpler into 2304972 on apache:master.

@asfgit asfgit merged commit d42f633 into apache:master Jun 20, 2017
asfgit pushed a commit that referenced this pull request Jun 20, 2017
@iemejia
Copy link
Member Author

iemejia commented Jun 20, 2017

Thanks for the review/ideas Eugene.

@iemejia iemejia deleted the BEAM-2411-make-hbaseio-write-simpler branch June 20, 2017 09:53
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.

5 participants