Skip to content

Fix race condition in Tables. Closes #768 #970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
milleruntime wants to merge 1 commit into from
Closed

Fix race condition in Tables. Closes #768 #970

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 2 additions & 19 deletions core/src/main/java/org/apache/accumulo/core/clientImpl/Tables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ public class Tables {
// frequently
private static Cache<String,TableMap> instanceToMapCache = CacheBuilder.newBuilder()
.expireAfterAccess(10, TimeUnit.MINUTES).build();
private static Cache<String,ZooCache> instanceToZooCache = CacheBuilder.newBuilder()
.expireAfterAccess(10, TimeUnit.MINUTES).build();

static {
SingletonManager.register(new SingletonService() {
Expand All @@ -73,7 +71,6 @@ public synchronized void enable() {
public synchronized void disable() {
try {
instanceToMapCache.invalidateAll();
instanceToZooCache.invalidateAll();
} finally {
enabled = false;
}
Expand All @@ -95,28 +92,14 @@ public static TableId getTableId(ClientContext context, String tableName)
}
}

/**
* Return the cached ZooCache for provided context. ZooCache is initially created with a watcher
* that will clear the TableMap cache for that instance when WatchedEvent occurs.
*/
private static ZooCache getZooCache(final ClientContext context) {
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(TABLES_PERMISSION);
}

final String uuid = context.getInstanceID();

try {
return instanceToZooCache.get(uuid, () -> {
final String zks = context.getZooKeepers();
final int timeOut = context.getZooKeepersSessionTimeOut();
return new ZooCacheFactory().getZooCache(zks, timeOut,
watchedEvent -> instanceToMapCache.invalidate(uuid));
Copy link
Copy Markdown
Contributor

@keith-turner keith-turner Feb 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is very important, it clears the map when zoocache changes.

Copy link
Copy Markdown
Contributor Author

@milleruntime milleruntime Feb 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh right. The TableMap in another process won't get updated.

Copy link
Copy Markdown
Contributor

@keith-turner keith-turner Feb 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #973 as an alternate way to do this. It was too hard to explain and easier to write the code.

});
} catch (ExecutionException e) {
throw new RuntimeException(e);
}
return new ZooCacheFactory().getZooCache(context.getZooKeepers(),
context.getZooKeepersSessionTimeOut());
}

/**
Expand Down