> On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote: > > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java, > > line 162 > > <https://reviews.apache.org/r/41506/diff/1/?file=1173734#file1173734line162> > > > > This can be a static string. We can create it once and reuse. Same can > > be follwed for other instances too.
Dropping . This string is already Internalized. > On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote: > > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java, > > line 207 > > <https://reviews.apache.org/r/41506/diff/1/?file=1173734#file1173734line207> > > > > driverendtime can be skipped from ordering construct. > > Rajat Khandelwal wrote: > Added it for a currently impossible (but plausible) bug, when driver > start time somehow gets missed while updating in the table, but driver end > time is updated correctly. Keeping both in order by will allow the attempts > to be ordered. Let me know if you still feel it should be removed. Should we have a separate column then , something like attempt_num. It ll be more clear when we look at the DB Table. > On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 834 > > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line834> > > > > Shall we let the driver decide on number of re-tries ? It can be diff > > for Hive and JDBC. Eventually (in future) it can even differ based on error > > type. > > Rajat Khandelwal wrote: > Sure, but I believe the server should impose some upper bound on retries. > That's why I've kept default retries to 3. That's server's bound, the drivers > have full freedom apart from that. Not sure if server should restrict the driver. A driver may decide for a particualr query/error it can try more than 3 times. The driver's shouldRetry() method can decide in that case ? > On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 1857 > > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line1857> > > > > Should we store numOfAttempts in finishedQueries table? If the value > > is zero, we need not check failed attempts table at all. Also admin can > > query finished query table and gets attempts statistics easily. > > > > Or is there a way to add a new Query in lensServerDao which joins > > finished queries table with failed attempts and gets the result in one hop. > > It can be messy though. > > Rajat Khandelwal wrote: > I don't think it'll be messy. `select * from finished_queries where > handle='handle'` will now be written as `select *, (count(*) from > failed_attempts where handle='handle') failed_attempts from finished_queries > where handle='handle'` > > Though it *will* be messy without the handle filter which it will still > be even after addition of the `num_attempts` field. I meant that can we can query the DB just once for 99% cases where attempts are 0 ? As of now we have to query twice in all cases. Once finished queries table and once fialed attempts table. - Puneet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41506/#review111459 ----------------------------------------------------------- On Dec. 21, 2015, 9:15 a.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41506/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2015, 9:15 a.m.) > > > Review request for lens. > > > Bugs: LENS-899 > https://issues.apache.org/jira/browse/LENS-899 > > > Repository: lens > > > Description > ------- > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java > 91cbe391a2bc81edae587b268104940122993243 > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > 88e5a01e3f08b12ff9e872d7412496dfc70ea626 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java > ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java > 79d8bf1e8f9798714d14e5a0336e24a148c4ca27 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java > 3d38ddd2dfe596b348bb48807aac5caeab5e936a > > lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java > a57a6e4ed1827ee5094cbfd423c183766d9dc5f4 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 1269e45ff7e5b9392108e338735b9774fb8a2686 > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java > d8e654d72989d7a8f9334adc81b82a9e0810ee39 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 2dff9afeb384738fa21ef7381cb5a54e3c02165a > > Diff: https://reviews.apache.org/r/41506/diff/ > > > Testing > ------- > > > Thanks, > > Rajat Khandelwal > >
