> On Jan. 25, 2018, 12:35 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java > > Line 231 (original), 246 (patched) > > <https://reviews.apache.org/r/65268/diff/3/?file=1944703#file1944703line246> > > > > Since you are switching to time-based max it becomes the hard line. You > > want to either limit number of retries or total number but not both at the > > same time.
Configuration to limit the retry count is already avaialable in the released versions of sentry. It's not a good idea to remove it. We do not know what is the time that would take for a transaction to be succsfull in the event of JDO failure that is why time based limit is set to some higher number so that it will not be hit in normal cases unless some configured ridiculously high retry count or has configured high retry time. > On Jan. 25, 2018, 12:35 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java > > Line 247 (original), 268 (patched) > > <https://reviews.apache.org/r/65268/diff/3/?file=1944703#file1944703line268> > > > > This may make total sleep time greater then the allowed sleep time. You > > need to make sure that you never sleep longer then the remaining time left. In each iteration sleep time is calculated for next iteration. Logic added at line 255 will make sure that the total sleep time never exceeds max allowed sleep time. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65268/#review196187 ----------------------------------------------------------- On Jan. 23, 2018, 11:40 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65268/ > ----------------------------------------------------------- > > (Updated Jan. 23, 2018, 11:40 p.m.) > > > Review request for sentry, Arjun Mishra, Na Li, Sergio Pena, and Vadim > Spector. > > > Bugs: SENTRY-1904 > https://issues.apache.org/jira/browse/SENTRY-1904 > > > Repository: sentry > > > Description > ------- > > The TransactionManager uses exponential backoff strategy for transaction > retries. This may cause some transactions to be delayed by a very long time. > We should also have a constraint on the max time for a transaction so that we > do not retry for too long. > > New patch that is attached adds upper bounds on below > > 1.Interval between the retry attempts which increases exponentially. > 2.Total time a transaction could spend in retries. > > > With out these limits we would not have a control on how long a transaction > could be be active. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java > 2f2b98412e7dfdcc847ffe7975a70f452554e747 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java > f4ff962a67f8a5c23cc5c8daa7bcb861d2e6b6a5 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 7e02874b4be6a7109108809b1f404fe971b7b8e2 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > b4100278392986c161625a366212c6fef66ec0a9 > > > Diff: https://reviews.apache.org/r/65268/diff/3/ > > > Testing > ------- > > Made sure all the tests pass. > > > Thanks, > > kalyan kumar kalvagadda > >
