alex-ninja commented on a change in pull request #1208:
URL: https://github.com/apache/cassandra/pull/1208#discussion_r711606822
##########
File path: src/java/org/apache/cassandra/auth/Roles.java
##########
@@ -38,26 +35,8 @@
private static final Role NO_ROLE = new Role("", false, false,
Collections.emptyMap(), Collections.emptySet());
- private static RolesCache cache;
- static
- {
- initRolesCache(DatabaseDescriptor.getRoleManager(),
- () ->
DatabaseDescriptor.getAuthenticator().requireAuthentication());
- }
-
- @VisibleForTesting
- public static void initRolesCache(IRoleManager roleManager,
BooleanSupplier enableCache)
- {
- if (cache != null)
- cache.unregisterMBean();
- cache = new RolesCache(roleManager, enableCache);
- }
-
- @VisibleForTesting
- public static void clearCache()
- {
- cache.invalidate();
- }
+ public static final RolesCache cache = new
RolesCache(DatabaseDescriptor.getRoleManager(),
Review comment:
Similarly to other caches it is setup in tests during initialization
instead of changing the behavior via "VisibleForTesting methods". Here I
ensured the cache instance is immutable (stored in a static final variable), so
it is safe to expose.
##########
File path: src/java/org/apache/cassandra/auth/PasswordAuthenticator.java
##########
@@ -69,14 +69,18 @@
static final byte NUL = 0;
private SelectStatement authenticateStatement;
- private CredentialsCache cache;
+ private static CredentialsCache cache;
// No anonymous access.
public boolean requireAuthentication()
{
return true;
}
+ public static CredentialsCache getCredentialsCache() {
Review comment:
Need to discuss this and decided how we want to deal with this. From one
hand, it exposes the cache outside and a kind of breaks encapsulation. From
another hand, I really need the access to the cache from outside. Theoretically
I could expose some particular methods, but:
1. There are many methods to expose
- invalidate()
- invalidate(key)
- getAll()
- get(key)
2. exposing some cache-related methods from PasswordAuthenticator/Roles/etc
classes seems to be a bit confusing
The same problem is actual for ALL caches. For now, I made all of them
public, so I could access them. But we need to decided what we want to do -
keep it as is / expose methods partially / make some structural changes (not
sure what exactly).
The current implementation with public access to the static caches still
seems to be same since the caches are thread-safe and immutable (I mean the
static instance variables - aka constants).
I'd be happy to hear any suggestions.
##########
File path: test/unit/org/apache/cassandra/auth/RolesTest.java
##########
@@ -40,14 +46,12 @@
public static void setupClass()
{
SchemaLoader.prepareServer();
- // create the system_auth keyspace so the IRoleManager can function as
normal
- SchemaLoader.createKeyspace(SchemaConstants.AUTH_KEYSPACE_NAME,
- KeyspaceParams.simple(1),
-
Iterables.toArray(AuthKeyspace.metadata().tables, TableMetadata.class));
-
- IRoleManager roleManager = new LocalCassandraRoleManager();
- roleManager.setup();
- Roles.initRolesCache(roleManager, () -> true);
+ LocalCassandraRoleManager roleManager = new
LocalCassandraRoleManager();
Review comment:
That's the right way to initialize this test, so there won't be a need
for `Roles.initRolesCache` method.
##########
File path:
src/java/org/apache/cassandra/db/virtual/AbstractMutableVirtualTable.java
##########
@@ -145,7 +145,7 @@ protected void applyRangeTombstone(ColumnValues
partitionKey, Range<ColumnValues
throw invalidRequest("Range deletion is not supported by table %s",
metadata);
}
- protected void applyRowDeletion(ColumnValues partitionKey, ColumnValues
clusteringColumnValues)
+ protected void applyRowDeletion(ColumnValues partitionKey, ColumnValues
clusteringColumns)
Review comment:
Missed that on review, so fixing now.
##########
File path: doc/source/new/virtualtables.rst
##########
@@ -308,6 +324,56 @@ As another example, to find how much time is remaining for
SSTable tasks, use th
SELECT total - progress AS remaining
FROM system_views.sstable_tasks;
+Auth Caches Virtual Tables
+****************************
+
+Every authentication cache has a separate virtual table associated. The
virtual tables show the data stored in caches
+and additionally support DELETE and TRUNCATE operations. In fact these
operations just invalidate data in caches, no
+data is actually deleted from real tables.
+
+The tables show the following information:
+
+::
+
+ cqlsh:system_views> select * from credentials_cache;
Review comment:
I have a general concern on the current structure of this page - it is
hard to maintain it (keep up-to-date). Maybe we should not list all table here?
It won't be a part of this ticket for sure, I just want to raise the question...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]