> On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java > > Lines 426 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011252#file2011252line429> > > > > This seems to switch the test from 1 api to another. Could it be > > parametrized so the old one doesn't loose coverage?
Parametrized. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/AbstractRecordWriter.java > > Lines 113 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011258#file2011258line127> > > > > Is there a plan to support MM tables? (with batch size of 1 it can be > > done). They won't implement AcidOutputFormat Will take this up in a follow up. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java > > Lines 76 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011264#file2011264line76> > > > > 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()) Updated javadoc. Currently event close() and abortTransaction() are not thread safe. If a separate thread has to close or abort then it has to coordinate via external variables. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java > > Lines 283 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011264#file2011264line283> > > > > 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. I changed the default batch size to 1. Also marked this API as evolving with a note. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java > > Lines 377 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011264#file2011264line377> > > > > 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. This was retained from old code. Updated to use add_partition() which make is simpler and efficient. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java > > Lines 399 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011264#file2011264line399> > > > > use Warehouse.makePartName()? Note required anymore. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java > > Lines 996 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011264#file2011264line996> > > > > "we don'table wait" Some weird IDE rename all over. Fixed it. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/HiveStreamingConnection.java > > Lines 1050 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011264#file2011264line1050> > > > > is this for testing? How can the user control these? Made it user configurable with a note advising against changing it. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/RecordWriter.java > > Lines 53 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011274#file2011274line53> > > > > maybe make this onNewBatch() since it's not starting a batch Removed this API. since the min and max write id doesn't change for a transaction batch, I am passing them via init() API now. > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/java/org/apache/hive/streaming/RecordWriter.java > > Line 42 (original), 60 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011274#file2011274line60> > > > > close() or closeWriter()? Renamed it to close() > On April 26, 2018, 2:09 a.m., Eugene Koifman wrote: > > streaming/src/test/org/apache/hive/streaming/TestStreamingDynamicPartitioning.java > > Lines 261 (patched) > > <https://reviews.apache.org/r/66645/diff/7/?file=2011287#file2011287line261> > > > > why does it need bucketid? Not required fixed. - Prasanth_J ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66645/#review201965 ----------------------------------------------------------- On April 24, 2018, 9: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, 9: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 > >
