> On May 18, 2015, 8:08 a.m., Rajat Khandelwal wrote: > > lens-server-api/src/main/java/org/apache/lens/server/api/common/Constant.java, > > line 21 > > <https://reviews.apache.org/r/34210/diff/2/?file=959324#file959324line21> > > > > Rename this to something other than the all-generic `Constant`. It > > doesn't yet contain every possible `Constant` to be eligible for the name > > `Constant`.
Couldn't figure out anything common between REQUEST_ID and LOG_SEGREGATION_ID to assign a common name to them. Please feel free to suggest a name which can relate these two constants. Putting both of them in separate enums will be enum bloat. Hence started a generic common Constant enum in lens-server-api which doesn't exist right now. > On May 18, 2015, 8:08 a.m., Rajat Khandelwal wrote: > > lens-server/src/main/java/org/apache/lens/server/InitialRequestFilter.java, > > line 41 > > <https://reviews.apache.org/r/34210/diff/2/?file=959328#file959328line41> > > > > It's not really a filter, going by the definition of `filter`. It's > > merely an `interceptor`. Let's rename it to something like `request > > enricher` or `request id appender` etc. Jersey has a notion in which request and response can be worked upon using container request and response filters. The interfaces are named ContainerRequestFilter and ContainerResponseFilter. Hence keeping the name as filter to align with existing intuition. Initial keyword is used with purpose as this is the first filter to be called among all filters when a request is processed by container. > On May 18, 2015, 8:08 a.m., Rajat Khandelwal wrote: > > lens-server/src/main/java/org/apache/lens/server/LensApplication.java, line > > 78 > > <https://reviews.apache.org/r/34210/diff/2/?file=959329#file959329line78> > > > > We can probably make it configurable, whether to add this filter or not. Not required. Things can be made configurable based on their need. Making everything configurable creates a maintainence overhead in configuration files. > On May 18, 2015, 8:08 a.m., Rajat Khandelwal wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 849 > > <https://reviews.apache.org/r/34210/diff/2/?file=959332#file959332line849> > > > > What happens when multiple threads(e.g. one submitter thread, one > > purger thread) both try to do `MDC.put`? Both thread works on their own MDC context. So both of them modifies their own MDC context. > On May 18, 2015, 8:08 a.m., Rajat Khandelwal wrote: > > lens-server/src/test/resources/log4j.properties, line 34 > > <https://reviews.apache.org/r/34210/diff/2/?file=959335#file959335line34> > > > > shouldn't we also add request id? Query handle, request id and run id are acting as log segregation id in different contexts. Query handle is preferred over request id which is preferred of run id. - Himanshu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34210/#review84119 ----------------------------------------------------------- On May 14, 2015, 2:06 p.m., Himanshu Gahlaut wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34210/ > ----------------------------------------------------------- > > (Updated May 14, 2015, 2:06 p.m.) > > > Review request for lens. > > > Bugs: LENS-514 > https://issues.apache.org/jira/browse/LENS-514 > > > Repository: lens > > > Description > ------- > > LENS-514: Adding unique identifier to log lines for log segregation > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/query/QueryHandle.java > 7b615e6216d7a31a743416dd839a63517be88e1f > lens-api/src/main/java/org/apache/lens/api/query/QueryPrepareHandle.java > efa04f1c7e7fddbd9176922425c66057f4be3089 > lens-api/src/main/java/org/apache/lens/api/response/LensResponse.java > f6c3593a131ab6779186be014d79b9f4c4cb9534 > > lens-server-api/src/main/java/org/apache/lens/server/api/common/Constant.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/PreparedQueryContext.java > 1ce89ac66fa817b3e47482a81b040259d2194477 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 169ac8dec4b98a0cab1f652fe85afe9737f02e39 > lens-server/src/main/java/org/apache/lens/server/AuthenticationFilter.java > b64d8223727d755aad95dee288f9561b767a5448 > lens-server/src/main/java/org/apache/lens/server/InitialRequestFilter.java > PRE-CREATION > lens-server/src/main/java/org/apache/lens/server/LensApplication.java > cb452e83e6edbe4f13a3f29b4ce0cbe45effcde3 > lens-server/src/main/java/org/apache/lens/server/LensServer.java > c6d7ea154f4c3f78eaa0675d99364a311351bf4c > lens-server/src/main/java/org/apache/lens/server/query/QueryApp.java > 0d23726a895a11208249b7ce458cb34e2265feeb > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 3bf180c9ab6f2a9fec6259848ebf65ecd147cfcf > > lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java > 9b6d6bc223069b28367d789db847cbf05ed386c7 > > lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java > 5e135be0348068125b69016274e00b01140c4f21 > lens-server/src/test/resources/log4j.properties > 65d065fa699f9797dcb13580359c4d41cfc52e9d > tools/conf-pseudo-distr/server/log4j.properties > afadc2f2856e7723e23265553440984bf2859869 > tools/conf/server/log4j.properties afadc2f2856e7723e23265553440984bf2859869 > > Diff: https://reviews.apache.org/r/34210/diff/ > > > Testing > ------- > > Testing done by executing query from lens-cli and verifying lens-server logs. > > Running test suite. Will update test results. > > > Thanks, > > Himanshu Gahlaut > >
