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]

Reply via email to