Skip to content

Commit c59e20f

Browse files
Zeta201Zeta201
committed
Refactor cache operations to use read-safe methods for concurrent environments
1 parent bb66016 commit c59e20f

21 files changed

+779
-51
lines changed

‎core/org.wso2.carbon.registry.core/src/main/java/org/wso2/carbon/registry/core/jdbc/dao/JDBCPathCache.java‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,74 @@ public String getPath(AbstractConnection conn, int id) throws SQLException {
449449
}
450450
}
451451

452+
/**
453+
* Method to get the path of a given path id.
454+
* <p>
455+
* Add a cache entry during a READ operation.
456+
* <p>
457+
* This populates the cache only if the key does not already have a value.
458+
* If a value already exists, the cache is left unchanged, which avoids
459+
* unnecessary cache invalidation broadcasts in clustered environments.
460+
*
461+
* @param conn the database connection to use.
462+
* @param id the path.
463+
*
464+
* @return the path corresponding to the given path id.
465+
* @throws SQLException if an error occurs while obtaining the path id.
466+
*/
467+
public String getPathOnRead(AbstractConnection conn, int id) throws SQLException {
468+
469+
String connectionId;
470+
if (conn != null && conn.getConnectionId() != null) {
471+
connectionId = conn.getConnectionId();
472+
} else {
473+
throw new SQLException("Connection is null");
474+
}
475+
RegistryCacheKey key =
476+
RegistryUtils.buildRegistryCacheKey(connectionId,
477+
CurrentSession.getTenantId(), Integer.toString(id));
478+
Cache<RegistryCacheKey, RegistryCacheEntry> cache = getCache();
479+
RegistryCacheEntry result = cache.get(key);
480+
if (result != null) {
481+
return result.getPath();
482+
} else {
483+
PreparedStatement ps = null;
484+
ResultSet results = null;
485+
try {
486+
String sql =
487+
"SELECT REG_PATH_VALUE FROM REG_PATH WHERE REG_PATH_ID=? AND REG_TENANT_ID=?";
488+
489+
ps = conn.prepareStatement(sql);
490+
ps.setInt(1, id);
491+
ps.setInt(2, CurrentSession.getTenantId());
492+
493+
results = ps.executeQuery();
494+
if (results.next()) {
495+
result = new RegistryCacheEntry(results.getString(DatabaseConstants.PATH_VALUE_FIELD));
496+
cache.putOnRead(key, result);
497+
return result.getPath();
498+
}
499+
500+
} finally {
501+
try {
502+
try {
503+
if (results != null) {
504+
results.close();
505+
}
506+
} finally {
507+
if (ps != null) {
508+
ps.close();
509+
}
510+
}
511+
} catch (SQLException ex) {
512+
String msg = RegistryConstants.RESULT_SET_PREPARED_STATEMENT_CLOSE_ERROR;
513+
log.error(msg, ex);
514+
}
515+
}
516+
return null;
517+
}
518+
}
519+
452520
/**
453521
* Method to get the path id of a given path.
454522
*
@@ -539,6 +607,102 @@ public int getPathID(AbstractConnection conn, String path) throws SQLException {
539607
return -1;
540608
}
541609

610+
/**
611+
* Method to get the path id of a given path.
612+
* <p>
613+
* Add a cache entry during a READ operation.
614+
* <p>
615+
* This populates the cache only if the key does not already have a value.
616+
* If a value already exists, the cache is left unchanged, which avoids
617+
* unnecessary cache invalidation broadcasts in clustered environments.
618+
*
619+
* @param conn the database connection to use.
620+
* @param path the path.
621+
*
622+
* @return the path id corresponding to the given path.
623+
* @throws SQLException if an error occurs while obtaining the path id.
624+
*/
625+
public int getPathIDOnRead(AbstractConnection conn, String path) throws SQLException {
626+
String connectionId = null;
627+
if (conn != null && conn.getConnectionId() != null) {
628+
connectionId = conn.getConnectionId();
629+
} else {
630+
throw new SQLException("Connection is null");
631+
}
632+
RegistryCacheKey key =
633+
RegistryUtils.buildRegistryCacheKey(connectionId,
634+
CurrentSession.getTenantId(), path);
635+
Cache<RegistryCacheKey,RegistryCacheEntry> cache = getCache();
636+
RegistryCacheEntry result = (RegistryCacheEntry) cache.get(key);
637+
638+
// TODO: FIX: Path Cache should only be updated if the key yields a valid registry path.
639+
// Recently, this has lead to:
640+
// org.wso2.carbon.registry.core.exceptions.RegistryException: Failed to add resource to
641+
// path /_system. Cannot add or update a child row: a foreign key constraint fails
642+
// (`stratos_db`.`REG_RESOURCE`, CONSTRAINT `REG_RESOURCE_FK_BY_PATH_ID` FOREIGN KEY
643+
// (`REG_PATH_ID`, `REG_TENANT_ID`) REFERENCES `REG_PATH` (`REG_PATH_ID`, `REG_TENANT_ID`))
644+
//
645+
// when registry separation is enabled. Thus, we need a better solution to address this, and
646+
// a better key which also contains the name of the DB in use. Un-comment the below once
647+
// this has been done.
648+
//
649+
// IMPORTANT: Never remove this comment until we are certain that the current fix is
650+
// actually working - Senaka.
651+
652+
if (result != null) {
653+
return result.getPathId();
654+
} else {
655+
ResultSet results = null;
656+
PreparedStatement ps = null;
657+
try {
658+
String sql =
659+
"SELECT REG_PATH_ID FROM REG_PATH WHERE REG_PATH_VALUE=? " +
660+
"AND REG_TENANT_ID=?";
661+
ps = conn.prepareStatement(sql);
662+
ps.setString(1, path);
663+
ps.setInt(2, CurrentSession.getTenantId());
664+
results = ps.executeQuery();
665+
666+
int pathId;
667+
if (results.next()) {
668+
pathId = results.getInt(DatabaseConstants.PATH_ID_FIELD);
669+
670+
if (pathId > 0) {
671+
RegistryCacheEntry e = new RegistryCacheEntry(pathId);
672+
cache.putOnRead(key, e);
673+
return pathId;
674+
}
675+
} else {
676+
//not found . set -1 in the cache as well for the path
677+
RegistryCacheEntry e = new RegistryCacheEntry(-1);
678+
cache.putOnRead(key, e);
679+
}
680+
681+
} catch (SQLException e) {
682+
String msg = "Failed to retrieving resource from " + path + ". " + e.getMessage();
683+
log.error(msg, e);
684+
throw e;
685+
} finally {
686+
try {
687+
try {
688+
if (results != null) {
689+
results.close();
690+
}
691+
} finally {
692+
if (ps != null) {
693+
ps.close();
694+
}
695+
}
696+
} catch (SQLException e) {
697+
String msg = RegistryConstants.RESULT_SET_PREPARED_STATEMENT_CLOSE_ERROR +
698+
e.getMessage();
699+
log.error(msg, e);
700+
}
701+
}
702+
}
703+
return -1;
704+
}
705+
542706
/**
543707
* Retrieves the existing path ID after rolling back an aborted transaction
544708
* caused by a constraint violation during concurrent path creation.

‎core/org.wso2.carbon.registry.core/src/main/java/org/wso2/carbon/registry/core/jdbc/dao/JDBCResourceDAO.java‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public ResourceIDImpl getResourceID(String path) throws RegistryException {
7272
try {
7373
// step1: need to check whether it is a resource or collection while retrieving path id
7474
JDBCPathCache pathCache = JDBCPathCache.getPathCache();
75-
int pathID = pathCache.getPathID(conn, path);
75+
int pathID = pathCache.getPathIDOnRead(conn, path);
7676
boolean isCollection = true;
7777
if (pathID == -1) {
7878
isCollection = false;
@@ -86,7 +86,7 @@ public ResourceIDImpl getResourceID(String path) throws RegistryException {
8686
String parentPath = RegistryUtils.getParentPath(path);
8787
resourceName = RegistryUtils.getResourceName(path);
8888

89-
pathID = pathCache.getPathID(conn, parentPath);
89+
pathID = pathCache.getPathIDOnRead(conn, parentPath);
9090
}
9191

9292
if (pathID != -1) {
@@ -118,7 +118,7 @@ public ResourceIDImpl getResourceID(String path, boolean isCollection)
118118
int pathID;
119119
String resourceName = null;
120120
if (isCollection) {
121-
pathID = JDBCPathCache.getPathCache().getPathID(conn, path);
121+
pathID = JDBCPathCache.getPathCache().getPathIDOnRead(conn, path);
122122
} else {
123123
// we have to re-get the path id for the parent path..
124124
String parentPath;
@@ -129,7 +129,7 @@ public ResourceIDImpl getResourceID(String path, boolean isCollection)
129129
}
130130
resourceName = RegistryUtils.getResourceName(path);
131131

132-
pathID = JDBCPathCache.getPathCache().getPathID(conn, parentPath);
132+
pathID = JDBCPathCache.getPathCache().getPathIDOnRead(conn, parentPath);
133133
}
134134

135135
if (pathID != -1) {
@@ -2202,6 +2202,25 @@ public String getPathFromId(int pathId) throws RegistryException {
22022202
}
22032203
}
22042204

2205+
/**
2206+
* Add a cache entry during a READ operation.
2207+
* <p>
2208+
* This populates the cache only if the key does not already have a value.
2209+
* If a value already exists, the cache is left unchanged, which avoids
2210+
* unnecessary cache invalidation broadcasts in clustered environments.
2211+
*
2212+
*/
2213+
public String getPathFromIdOnRead(int pathId) throws RegistryException {
2214+
try {
2215+
return JDBCPathCache.getPathCache()
2216+
.getPathOnRead(JDBCDatabaseTransaction.getConnection(), pathId);
2217+
} catch (SQLException e) {
2218+
String msg = "Failed to get the path for the path id " + pathId + ". " + e.getMessage();
2219+
log.error(msg, e);
2220+
throw new RegistryException(msg, e);
2221+
}
2222+
}
2223+
22052224
public String getPath(long version) throws RegistryException {
22062225
ResourceDO resourceDO = getResourceDO(version);
22072226
if (resourceDO == null) {
@@ -2212,7 +2231,7 @@ public String getPath(long version) throws RegistryException {
22122231

22132232
public String getPath(int pathId, String resourceName, boolean checkExistence)
22142233
throws RegistryException {
2215-
String pathCollection = getPathFromId(pathId);
2234+
String pathCollection = getPathFromIdOnRead(pathId);
22162235
if (pathCollection == null) {
22172236
return null;
22182237
}

‎core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/authorization/AuthorizationCache.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,39 @@ public void addToCache(String serverId, int tenantId, String userName,
123123
cache.put(key, cacheEntry);
124124
}
125125

126+
/**
127+
* Add a cache entry during a READ operation.
128+
* This populates the cache only if the key does not already have a value.
129+
* If a value already exists, the cache is left unchanged, which avoids
130+
* unnecessary cache invalidation broadcasts in clustered environments.
131+
* <p>
132+
* Adds an entry to the cache. Says whether given user or role is authorized
133+
* or not.
134+
*
135+
* @param serverId unique identifier for carbon server instance
136+
* @param userName Name of the user which was authorized. If this is null
137+
* roleName must not be null.
138+
* @param resourceId The resource on which user/role was authorized.
139+
* @param action The action which user/role authorized for.
140+
* @param isAuthorized Whether role/user was authorized or not. <code>true</code> for
141+
* authorized else <code>false</code>.
142+
*/
143+
public void addToCacheOnRead(String serverId, int tenantId, String userName,
144+
String resourceId, String action, boolean isAuthorized) {
145+
146+
if (!isCaseSensitiveUsername(userName, tenantId)) {
147+
userName = userName.toLowerCase();
148+
}
149+
150+
Cache<AuthorizationKey, AuthorizeCacheEntry> cache = this.getAuthorizationCache();
151+
if (isCacheNull(cache)) {
152+
return;
153+
}
154+
AuthorizationKey key = new AuthorizationKey(serverId, tenantId, userName, resourceId, action);
155+
AuthorizeCacheEntry cacheEntry = new AuthorizeCacheEntry(isAuthorized);
156+
cache.putOnRead(key, cacheEntry);
157+
}
158+
126159
/**
127160
* Looks up from cache whether given user is already authorized. If an entry
128161
* is not found throws an exception.

‎core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/authorization/JDBCAuthorizationManager.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ public boolean isUserAuthorized(String userName, String resourceId, String actio
287287
null, null,
288288
PermissionTreeUtil.toComponenets(resourceId));
289289
if (sr.getLastNodeAllowedAccess()) {
290-
authorizationCache.addToCache(cacheIdentifier, tenantId, userName, resourceId, action, true);
290+
authorizationCache.addToCacheOnRead(cacheIdentifier, tenantId, userName, resourceId, action, true);
291291
return true;
292292
}
293293

@@ -370,7 +370,7 @@ public boolean isUserAuthorized(String userName, String resourceId, String actio
370370
}
371371

372372
//need to add the authorization decision taken by role based permission
373-
authorizationCache.addToCache(cacheIdentifier, this.tenantId, userName, resourceId, action,
373+
authorizationCache.addToCacheOnRead(cacheIdentifier, this.tenantId, userName, resourceId, action,
374374
userAllowed);
375375

376376
if (log.isDebugEnabled()) {

0 commit comments

Comments
 (0)