----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66645/#review201965 -----------------------------------------------------------
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java Lines 426 (patched) <https://reviews.apache.org/r/66645/#comment283636> This seems to switch the test from 1 api to another. Could it be parametrized so the old one doesn't loose coverage? itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java Lines 753 (patched) <https://reviews.apache.org/r/66645/#comment283637> parametrize test instead? itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java Lines 823 (patched) <https://reviews.apache.org/r/66645/#comment283638> ame as above serde/src/java/org/apache/hadoop/hive/serde2/JsonSerDe.java Lines 148 (patched) <https://reviews.apache.org/r/66645/#comment283633> obsolte comment streaming/src/java/org/apache/hive/streaming/AbstractRecordWriter.java Line 82 (original), 95 (patched) <https://reviews.apache.org/r/66645/#comment283578> should this throw if this.conn != null to make sure Writers are not shared streaming/src/java/org/apache/hive/streaming/AbstractRecordWriter.java Lines 113 (patched) <https://reviews.apache.org/r/66645/#comment283577> Is there a plan to support MM tables? (with batch size of 1 it can be done). They won't implement AcidOutputFormat streaming/src/java/org/apache/hive/streaming/AbstractRecordWriter.java Lines 399 (patched) <https://reviews.apache.org/r/66645/#comment283583> presumably it also already exist for dynamic parts? streaming/src/java/org/apache/hive/streaming/AbstractRecordWriter.java Lines 402 (patched) <https://reviews.apache.org/r/66645/#comment283584> the log msg seems a bit misleading since createPartitionIfNotExists() may or may not create anything. Perhaps createPartitionIfNotExists() should return status if it actually crated something or do the logging in that method. streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java Lines 76 (patched) <https://reviews.apache.org/r/66645/#comment283585> It should explain the threading model somewhere. I assume everything is meant to be single threaded, though it's common for client code to process cancel type of event in a separate thread. here it would presumably map to close() (perhaps abortTransaction()) streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java Lines 79 (patched) <https://reviews.apache.org/r/66645/#comment283572> "bind" rather than "build"? streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java Lines 283 (patched) <https://reviews.apache.org/r/66645/#comment283606> Not sure we want to expose it in such an obvious way. It may be better if there is a table prop for this or perhaps the doc should say that this is 'advisory' and we may ignore the value. Longer term we'd like to consider an option where we force batch size = 1. This is the only option for some cloud stores and generally simplifies acid if we don't have deltas_x_y with x<>y which may contain aborted txns. streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java Lines 377 (patched) <https://reviews.apache.org/r/66645/#comment283582> why does this need Alter Table? You already have a metastore connection and you know that you are adding a partiton w/o data. In other words, why not just msClient.add_partition() and handle AlreadyExistsException or add_partitions( List<Partition> partitions, boolean ifNotExists, boolean needResults). This seems simpler and more efficient. streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java Lines 399 (patched) <https://reviews.apache.org/r/66645/#comment283601> use Warehouse.makePartName()? streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java Lines 996 (patched) <https://reviews.apache.org/r/66645/#comment283587> "we don'table wait" streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java Lines 1050 (patched) <https://reviews.apache.org/r/66645/#comment283600> is this for testing? How can the user control these? streaming/src/java/org/apache/hive/streaming/RecordWriter.java Lines 53 (patched) <https://reviews.apache.org/r/66645/#comment283634> maybe make this onNewBatch() since it's not starting a batch streaming/src/java/org/apache/hive/streaming/RecordWriter.java Line 42 (original), 60 (patched) <https://reviews.apache.org/r/66645/#comment283635> close() or closeWriter()? streaming/src/test/org/apache/hive/streaming/TestStreamingDynamicPartitioning.java Lines 261 (patched) <https://reviews.apache.org/r/66645/#comment283618> why does it need bucketid? - Eugene Koifman On April 24, 2018, 2:43 p.m., Prasanth_J wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66645/ > ----------------------------------------------------------- > > (Updated April 24, 2018, 2:43 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 f40c606 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java > 82ba775 > 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/lockmgr/DbTxnManager.java 7b7fd5d > serde/src/java/org/apache/hadoop/hive/serde2/JsonSerDe.java PRE-CREATION > 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/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 3343d10 > > streaming/src/test/org/apache/hive/streaming/TestStreamingDynamicPartitioning.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66645/diff/7/ > > > Testing > ------- > > > Thanks, > > Prasanth_J > >
