> On May 5, 2017, 12:54 a.m., Sahil Takiar wrote: > > common/src/java/org/apache/hive/common/util/RetryUtilities.java > > Lines 25 (patched) > > <https://reviews.apache.org/r/58936/diff/1/?file=1706349#file1706349line25> > > > > Might want to looking https://github.com/rholder/guava-retrying > > Sahil Takiar wrote: > look into* > > Vihang Karajgaonkar wrote: > Thanks for the pointer. Took a quick look. It has some interesting ideas > but it doesn't seem to support reducing the workload size exponentially. It > has a exponential backoff retry interval, but not the in terms of workload > sizing. Also, I have never used this library before. Is it popular and > production ready? The last commit on this library was 2 years ago. > > Sahil Takiar wrote: > Ah I think I misunderstood what this code was doing. I thought that the > retries are triggered using an exponential backoff mechanism, but it seems > that they are tried immediately, one after the other. Should be consider > adding exponential backoff to the timings of the retries. This can help if > the network is flaky for some reason. Also, I think that guava-retrying > re-uses the same Callable object each time it retries the `call` method, so > the object could keep state about the number of times it has been retried and > use that to decay the batch size. > > I've used it before and it works pretty well. Other projects that use > this library can be found here: > https://mvnrepository.com/artifact/com.github.rholder/guava-retrying/2.0.0/usages > > Using this library isn't a blocking issue, just thought I would suggest > it in case it makes things simpler.
I tried to modify the patch for using this library and I realized quickly that it may not have much benefit. The issue is even if we implement a Callable which keeps the state like batch size and number of attempts, the Retryer doesn't tell the Callable object if it invoked the call() method due to the fact that there was an exception in the last attempt or if last attempt was successful but we still need to do some more work. In this case a such the Callable object will also have to keep track of if last attempt threw an exception or not. If we keep track of all these the code resembles very similar to what we have already in this patch. In my opinion using the library as of now might not give us much benefit. If in the future we have more complex scenarios we can consider using this one. What do you think? - Vihang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58936/#review173989 ----------------------------------------------------------- On May 2, 2017, 10:25 p.m., Vihang Karajgaonkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58936/ > ----------------------------------------------------------- > > (Updated May 2, 2017, 10:25 p.m.) > > > Review request for hive, Sergio Pena and Sahil Takiar. > > > Bugs: HIVE-16143 > https://issues.apache.org/jira/browse/HIVE-16143 > > > Repository: hive-git > > > Description > ------- > > HIVE-16143 : Improve msck repair batching > > > Diffs > ----- > > common/src/java/org/apache/hive/common/util/RetryUtilities.java > PRE-CREATION > common/src/test/org/apache/hive/common/util/TestRetryUtilities.java > PRE-CREATION > itests/hive-blobstore/src/test/queries/clientpositive/create_like.q > 38f384e4c547d3c93d510b89fccfbc2b8e2cba09 > itests/hive-blobstore/src/test/results/clientpositive/create_like.q.out > 0d362a716291637404a3859fe81068594d82c9e0 > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java > 2ae1eacb68cef6990ae3f2050af0bed7c8e9843f > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java > 917e565f28b2c9aaea18033ea3b6b20fa41fcd0a > > ql/src/test/org/apache/hadoop/hive/ql/exec/TestMsckCreatePartitionsInBatches.java > PRE-CREATION > ql/src/test/queries/clientpositive/msck_repair_0.q > 22542331621ca4ce5277c2f46a4264b7540a4d1e > ql/src/test/queries/clientpositive/msck_repair_1.q > ea596cbbd2d4c230f2b5afbe379fc1e8836b6fbd > ql/src/test/queries/clientpositive/msck_repair_2.q > d8338211e970ebac68a7471ee0960ccf2d51cba3 > ql/src/test/queries/clientpositive/msck_repair_3.q > fdefca121a2de361dbd19e7ef34fb220e1733ed2 > ql/src/test/queries/clientpositive/msck_repair_batchsize.q > e56e97ac36a6544f3e20478fdb0e8fa783a857ef > ql/src/test/results/clientpositive/msck_repair_0.q.out > 2e0d9dc423071ebbd9a55606f196cf7752e27b1a > ql/src/test/results/clientpositive/msck_repair_1.q.out > 3f2fe75b194f1248bd5c073dd7db6b71b2ffc2ba > ql/src/test/results/clientpositive/msck_repair_2.q.out > 3f2fe75b194f1248bd5c073dd7db6b71b2ffc2ba > ql/src/test/results/clientpositive/msck_repair_3.q.out > 3f2fe75b194f1248bd5c073dd7db6b71b2ffc2ba > ql/src/test/results/clientpositive/msck_repair_batchsize.q.out > ba99024163a1f2c59d59e9ed7ea276c154c99d24 > ql/src/test/results/clientpositive/repair.q.out > c1834640a35500c521a904a115a718c94546df10 > > > Diff: https://reviews.apache.org/r/58936/diff/1/ > > > Testing > ------- > > > Thanks, > > Vihang Karajgaonkar > >
