-----------------------------------------------------------
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
> 
>

Reply via email to