You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Eugene Koifman <ek...@hortonworks.com> on 2018/05/01 19:10:43 UTC

Re: Review Request 66645: HIVE-19211: New streaming ingest API and support for dynamic partitioning

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66645/#review202201
-----------------------------------------------------------




streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java
Line 490 (original), 414 (patched)
<https://reviews.apache.org/r/66645/#comment283952>

    should these be final?



streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java
Lines 442 (patched)
<https://reviews.apache.org/r/66645/#comment283953>

    what does this achieve?
    you said that this API doesn't support concurrency and begin/commit/close/ect have be sequntial.  That means that minTxnId of the batch can only change linearly.  So what does atomicReference buy you over 'volotile' for example?
    
    commitImpl, is not atomic - it calls msClient.commitTxn() and then adjusts minTxn.  But a hreartbeat between these 2 will end up heartbeating a committed txn...
    
    It seems that maxTxn never changes, and the only thing that needs to be updated in the HeartbeatRunnable is the minTxn which needs to be volatile.  
    
    Is there something else that this is trying to solve?



streaming/src/test/org/apache/hive/streaming/TestStreaming.java
Lines 589 (patched)
<https://reviews.apache.org/r/66645/#comment283950>

    followup jira?



hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java
Line 442 (original), 442 (patched)
<https://reviews.apache.org/r/66645/#comment283943>

    is there a follow up jira for this?



itests/hive-unit/pom.xml
Lines 79 (patched)
<https://reviews.apache.org/r/66645/#comment283944>

    what change requires this?



ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRecordUpdater.java
Lines 471 (patched)
<https://reviews.apache.org/r/66645/#comment283945>

    if you have only 1 txn in a batch, why call flush at all?  (this flush() is called when commit() is called) . Won't closign the file do the right thing?


- Eugene Koifman


On April 30, 2018, 4:10 p.m., Prasanth_J wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66645/
> -----------------------------------------------------------
> 
> (Updated April 30, 2018, 4:10 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Eugene Koifman.
> 
> 
> Bugs: HIVE-19211
>     https://issues.apache.org/jira/browse/HIVE-19211
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-19211: New streaming ingest API and support for dynamic partitioning
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6e35653 
>   hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java 90dbdac 
>   itests/hive-unit/pom.xml 3ae7f2f 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java 8ee033d 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveClientCache.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreUtils.java a66c135 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRecordUpdater.java 09f8802 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 76569d5 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4661881 
>   serde/src/java/org/apache/hadoop/hive/serde2/JsonSerDe.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java 8c159e9 
>   streaming/pom.xml b58ec01 
>   streaming/src/java/org/apache/hive/streaming/AbstractRecordWriter.java 25998ae 
>   streaming/src/java/org/apache/hive/streaming/ConnectionError.java 668bffb 
>   streaming/src/java/org/apache/hive/streaming/ConnectionInfo.java PRE-CREATION 
>   streaming/src/java/org/apache/hive/streaming/DelimitedInputWriter.java 898b3f9 
>   streaming/src/java/org/apache/hive/streaming/HeartBeatFailure.java b1f9520 
>   streaming/src/java/org/apache/hive/streaming/HiveEndPoint.java b04e137 
>   streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java PRE-CREATION 
>   streaming/src/java/org/apache/hive/streaming/ImpersonationFailed.java 23e17e7 
>   streaming/src/java/org/apache/hive/streaming/InvalidColumn.java 0011b14 
>   streaming/src/java/org/apache/hive/streaming/InvalidPartition.java f1f9804 
>   streaming/src/java/org/apache/hive/streaming/InvalidTable.java ef1c91d 
>   streaming/src/java/org/apache/hive/streaming/InvalidTransactionState.java PRE-CREATION 
>   streaming/src/java/org/apache/hive/streaming/InvalidTrasactionState.java 762f5f8 
>   streaming/src/java/org/apache/hive/streaming/PartitionCreationFailed.java 5f9aca6 
>   streaming/src/java/org/apache/hive/streaming/PartitionHandler.java PRE-CREATION 
>   streaming/src/java/org/apache/hive/streaming/PartitionInfo.java PRE-CREATION 
>   streaming/src/java/org/apache/hive/streaming/QueryFailedException.java ccd3ae0 
>   streaming/src/java/org/apache/hive/streaming/RecordWriter.java dc6d70e 
>   streaming/src/java/org/apache/hive/streaming/SerializationError.java a57ba00 
>   streaming/src/java/org/apache/hive/streaming/StreamingConnection.java 2f760ea 
>   streaming/src/java/org/apache/hive/streaming/StreamingException.java a7f84c1 
>   streaming/src/java/org/apache/hive/streaming/StreamingIOFailure.java 0dfbfa7 
>   streaming/src/java/org/apache/hive/streaming/StrictDelimitedInputWriter.java PRE-CREATION 
>   streaming/src/java/org/apache/hive/streaming/StrictJsonWriter.java 0077913 
>   streaming/src/java/org/apache/hive/streaming/StrictRegexWriter.java c0b7324 
>   streaming/src/java/org/apache/hive/streaming/TransactionBatch.java 2b05771 
>   streaming/src/java/org/apache/hive/streaming/TransactionBatchUnAvailable.java a8c8cd4 
>   streaming/src/java/org/apache/hive/streaming/TransactionError.java a331b20 
>   streaming/src/test/org/apache/hive/streaming/TestDelimitedInputWriter.java f0843a1 
>   streaming/src/test/org/apache/hive/streaming/TestStreaming.java 0ec3048 
>   streaming/src/test/org/apache/hive/streaming/TestStreamingDynamicPartitioning.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66645/diff/10/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Prasanth_J
> 
>