> On Sept. 5, 2017, 9:02 p.m., Vamsee Yarlagadda wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > > Line 115 (original), 122 (patched) > > <https://reviews.apache.org/r/62107/diff/1/?file=1816055#file1816055line127> > > > > What would prevent the execution from reaching this step and a full > > snapshot from HMS kicked off at the same time? > > > > Do you think we should have the block on both the sides? > > Brian Towles wrote: > Once it hits this point all of the other cases have been tried and the > full update should be available due to an unavailable sequence number > > Vamsee Yarlagadda wrote: > Agreed but my question is let's say HDFS request (after going through all > the condition checks) decides to send a full snapshot and there could be some > activity on HMSFollower side that can also start a full snapshot from HMS. > There is nothing that is stopping HMSFollower from doing that. > > During this time, the memory consumption will spike again because of both > the snapshots happening at the same time.
I see what you mean. I dont really feel there is a way to I dont think we ever want to stop the HMSFollower from doing a full update when nessesary. Since that is pulling from the source of truth and we can delay the requesting clients. Yes if we have a pull already in progress from HDFS and HMSFollower starts a full pull from HMS this could be an sizing issue. I dont know if its going to be a regession because this is already essentially what we had anyways by keeping the full HMS snapshot in memory anyways prior to 2.0. One of the major issues I think is more likly multiple fulls happening at the same time from the HDFS side. Which SENTRY-1919 was addressing. I really dont think we wan to start putting a "only one big action can run at a time" type of limiter on Sentry over something that probably should be addressed by a sysadmin and adding memory. Once we hit a large system like citi we are going to have to be able to scal eand that means memory requierments as well. Latency of an update across the whole chain needs to be kept in mind. We are already saying that if a full update is running we dont do a NN full update. If we start doing it the other way as well then we really have no idea what our "max lag" time for an acl update to appear on the NN is, the HMSFollower becomes dependent on possibly multiple NN's doing requests. Thoughts? - Brian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62107/#review184619 ----------------------------------------------------------- On Sept. 6, 2017, 6:33 p.m., Brian Towles wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62107/ > ----------------------------------------------------------- > > (Updated Sept. 6, 2017, 6:33 p.m.) > > > Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, Sergio > Pena, and Vadim Spector. > > > Bugs: SENTRY-1918 > https://issues.apache.org/jira/browse/SENTRY-1918 > > > Repository: sentry > > > Description > ------- > > Converted SentryService to a singleton and have HMSFollower setting a flag > when a full update is running. Set the DBUpdateForwarder to only send back > full updates when then full update is not running. Changed tests and added a > Helper to support the new singleton. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > 5d744214e14d6c48194b3a0bf84d6e10070b020a > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java > 8fbc10048003ab4b8a38676e241ae0fd27d2392c > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java > 4652dc9e0a35a20780418d8e388595198a8f9ccd > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > d4feb380fa0f3bd5f237609a107295a2d23eae3b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > d44abffc199cd46d3ab7d2230dfdb2075736a8f0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceFactory.java > 1685702c6dbc9715c8885a29a80bc68509550f0b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java > 8959ad8ec77a1268a755faf451d99a81d87cb889 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java > 3b3b30e0bdf65e774d854e8252339960afcdd306 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java > fe4164d4980738e8df6e93ee5f78e82e1d874ae1 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java > 32e67b9f8efbbec12e93794f0ab00d5b8ed555b1 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java > 6895720f6e6d5f12ce468d7368e0d0ee9a0fae88 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java > b416ef81db21dfcf0157b2947c53721d5bcdf560 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java > 4cfb1f74cd90dbdb7c1aa5be07729e68353dd501 > sentry-tests/sentry-tests-kafka/pom.xml > 56a3ef10a9071929776cb7211bdb8ead921deace > > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java > 7c459993efb811b7ee11430f42791d1083f8d9df > sentry-tests/sentry-tests-solr/pom.xml > c70476808688c80e1723d5e65e3b8cf6d1b64250 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java > ccea82ee8ee2b976f14bc6da46a84c764b3b96f9 > > sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java > 80f158a1fd626cdddeae9e2ee264f361d78bc2a7 > > > Diff: https://reviews.apache.org/r/62107/diff/5/ > > > Testing > ------- > > Unit Tests > > > Thanks, > > Brian Towles > >
