[jira] [Commented] (SENTRY-1683) MetastoreCacheInitializer has a race condition in handling results list
[
https://issues.apache.org/jira/browse/SENTRY-1683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15951841#comment-15951841
]
Hadoop QA commented on SENTRY-1683:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12861532/SENTRY-1683.001.patch
against master.
{color:green}Overall:{color} +1 all checks pass
{color:green}SUCCESS:{color} all tests passed
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2467/console
This message is automatically generated.
> MetastoreCacheInitializer has a race condition in handling results list
> ---
>
> Key: SENTRY-1683
> URL: https://issues.apache.org/jira/browse/SENTRY-1683
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: Alexander Kolbasov
> Attachments: SENTRY-1683.001.patch, SENTRY-1683.001.patch
>
>
> The {{MetastoreCacheInitializer}} has the following logic:
> 1) It creates a list of futures to hold results of executors ({{results}})
> 2) {{CreateInitialUpdate() }}does this:
> {code}
> UpdateableAuthzPaths createInitialUpdate() throws
> Exception {
> PathsUpdate tempUpdate = new PathsUpdate(-1, false);
> List allDbStr = hmsHandler.get_all_databases();
> for (String dbName : allDbStr) {
> Callable dbTask = new DbTask(tempUpdate, dbName);
> results.add(threadPool.submit(dbTask));
> }
> while (taskCounter.get() > 0) {
> Thread.sleep(1000);
> // Wait until no more tasks remain
> }
> for (Future result : results) {
> CallResult callResult = result.get();
> // Fail the HMS startup if tasks are not all successful and
> // fail on partial updates flag is set in the config.
> if (!callResult.getSuccessStatus() && failOnRetry) {
> throw callResult.getFailure();
> }
> }
> authzPaths.updatePartial(Lists.newArrayList(tempUpdate),
> new ReentrantReadWriteLock());
> return authzPaths;
> }
> {code}
> Note that in both cases the results list is accessed without holding any
> locks.
> But the list is also updated from tasks themselves:
> {code}
> class DbTask extends BaseTask {
> @Override
> public void doTask() throws TException, SentryMalformedPathException {
> ...
> List allTblStr = hmsHandler.get_all_tables(dbName);
> for (int i = 0; i < allTblStr.size(); i += maxTablesPerCall) {
> synchronized (results) {
> results.add(threadPool.submit(tableTask));
> }
> }
> {code}
> There is some synchronization which uses taskCounter so the second walk of
> the list happens when all tasks are complete, but the first walk isn't
> thread-safe - the list may be updated while initial tasks are created there.
> Note that some access are synchronized and some are not.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (SENTRY-1683) MetastoreCacheInitializer has a race condition in handling results list
[
https://issues.apache.org/jira/browse/SENTRY-1683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15950389#comment-15950389
]
Hadoop QA commented on SENTRY-1683:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12861378/SENTRY-1683.001.patch
against master.
{color:green}Overall:{color} +1 all checks pass
{color:green}SUCCESS:{color} all tests passed
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2462/console
This message is automatically generated.
> MetastoreCacheInitializer has a race condition in handling results list
> ---
>
> Key: SENTRY-1683
> URL: https://issues.apache.org/jira/browse/SENTRY-1683
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: Alexander Kolbasov
> Attachments: SENTRY-1683.001.patch
>
>
> The {{MetastoreCacheInitializer}} has the following logic:
> 1) It creates a list of futures to hold results of executors ({{results}})
> 2) {{CreateInitialUpdate() }}does this:
> {code}
> UpdateableAuthzPaths createInitialUpdate() throws
> Exception {
> PathsUpdate tempUpdate = new PathsUpdate(-1, false);
> List allDbStr = hmsHandler.get_all_databases();
> for (String dbName : allDbStr) {
> Callable dbTask = new DbTask(tempUpdate, dbName);
> results.add(threadPool.submit(dbTask));
> }
> while (taskCounter.get() > 0) {
> Thread.sleep(1000);
> // Wait until no more tasks remain
> }
> for (Future result : results) {
> CallResult callResult = result.get();
> // Fail the HMS startup if tasks are not all successful and
> // fail on partial updates flag is set in the config.
> if (!callResult.getSuccessStatus() && failOnRetry) {
> throw callResult.getFailure();
> }
> }
> authzPaths.updatePartial(Lists.newArrayList(tempUpdate),
> new ReentrantReadWriteLock());
> return authzPaths;
> }
> {code}
> Note that in both cases the results list is accessed without holding any
> locks.
> But the list is also updated from tasks themselves:
> {code}
> class DbTask extends BaseTask {
> @Override
> public void doTask() throws TException, SentryMalformedPathException {
> ...
> List allTblStr = hmsHandler.get_all_tables(dbName);
> for (int i = 0; i < allTblStr.size(); i += maxTablesPerCall) {
> synchronized (results) {
> results.add(threadPool.submit(tableTask));
> }
> }
> {code}
> There is some synchronization which uses taskCounter so the second walk of
> the list happens when all tasks are complete, but the first walk isn't
> thread-safe - the list may be updated while initial tasks are created there.
> Note that some access are synchronized and some are not.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (SENTRY-1683) MetastoreCacheInitializer has a race condition in handling results list
[
https://issues.apache.org/jira/browse/SENTRY-1683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15950292#comment-15950292
]
Alexander Kolbasov commented on SENTRY-1683:
A simple fix would be to use Vector instead of ArrayList.
> MetastoreCacheInitializer has a race condition in handling results list
> ---
>
> Key: SENTRY-1683
> URL: https://issues.apache.org/jira/browse/SENTRY-1683
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>
> The {{MetastoreCacheInitializer}} has the following logic:
> 1) It creates a list of futures to hold results of executors ({{results}})
> 2) {{CreateInitialUpdate() }}does this:
> {code}
> UpdateableAuthzPaths createInitialUpdate() throws
> Exception {
> PathsUpdate tempUpdate = new PathsUpdate(-1, false);
> List allDbStr = hmsHandler.get_all_databases();
> for (String dbName : allDbStr) {
> Callable dbTask = new DbTask(tempUpdate, dbName);
> results.add(threadPool.submit(dbTask));
> }
> while (taskCounter.get() > 0) {
> Thread.sleep(1000);
> // Wait until no more tasks remain
> }
> for (Future result : results) {
> CallResult callResult = result.get();
> // Fail the HMS startup if tasks are not all successful and
> // fail on partial updates flag is set in the config.
> if (!callResult.getSuccessStatus() && failOnRetry) {
> throw callResult.getFailure();
> }
> }
> authzPaths.updatePartial(Lists.newArrayList(tempUpdate),
> new ReentrantReadWriteLock());
> return authzPaths;
> }
> {code}
> Note that in both cases the results list is accessed without holding any
> locks.
> But the list is also updated from tasks themselves:
> {code}
> class DbTask extends BaseTask {
> @Override
> public void doTask() throws TException, SentryMalformedPathException {
> ...
> List allTblStr = hmsHandler.get_all_tables(dbName);
> for (int i = 0; i < allTblStr.size(); i += maxTablesPerCall) {
> synchronized (results) {
> results.add(threadPool.submit(tableTask));
> }
> }
> {code}
> There is some synchronization which uses taskCounter so the second walk of
> the list happens when all tasks are complete, but the first walk isn't
> thread-safe - the list may be updated while initial tasks are created there.
> Note that some access are synchronized and some are not.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
