[
https://issues.apache.org/jira/browse/CASSANDRA-7840?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14118415#comment-14118415
]
Yuki Morishita commented on CASSANDRA-7840:
-------------------------------------------
Thanks.
I looked through the all patches, and am still not sure singleton instances of
Factories/Managers(KeyspaceManager, KSMetaDataFactory, ...) are good. Maybe
because I don't see how those turn out in the later steps. I feel we can just
leave them (for now) as they are since those are still the group of convinient
methods and do not have their own states.
Related,
bq. Moving the SystemKeyspace CFMetaData definitions off of the CFMetaData
class may end up causing more problems than it solves, so I may end up
reverting it.
I'm +1 for not moving, for now.
My comments on other classes are as follows. Overall, there are unused imports
because of the refactoring, so make sure to clean them up.
BatchlogManager
* can make batchlogTasks private and add method to shut it down.
QueryProcessor
* internalQueryState is better to be cached as before.
SinkManager
* Can be instance variable of MessagingService, but can do in later step.
Auth
* superuserSetupDelay can be left static, or if you make it instance var,
then I prefer to be constructor param as well.
SystemKeyspace
* SystemKeyspace.instance access from itself.
StorageProxy
* Not sure the impact on {{@Inline}}-ed static methods move to instance
methods. (cc [~benedict], [~tjake])
* StorageProxy.instance access from inner classes where storageProxy local
instance is available
StorageService
* RING_DELAY can be left static
* StorageProxy.instance.instance at L562
* StorageService.instance access from itself
DatabaseDescriptor
* constructor is assining Config object created to wrong var when
clientMode (should be {{this.conf = new Config()}} instead of {{conf = new
Config}}).
* should we keep clientMode boolean? it is not accessed outside of
constructor.
> Refactor static state & functions into static singletons
> --------------------------------------------------------
>
> Key: CASSANDRA-7840
> URL: https://issues.apache.org/jira/browse/CASSANDRA-7840
> Project: Cassandra
> Issue Type: Sub-task
> Reporter: Blake Eggleston
> Labels: patch
> Attachments:
> 0001-splitting-StorageService-executors-into-a-separate-c.patch,
> 0002-making-DatabaseDescriptor-a-singleton.patch,
> 0003-refactoring-StorageService-static-methods.patch,
> 0004-making-StorageProxy-a-singleton.patch,
> 0005-making-MigrationManager-a-singleton.patch,
> 0006-making-SystemKeyspace-a-singleton.patch,
> 0007-making-Auth-a-singleton.patch,
> 0008-removing-static-methods-and-initialization-from-Comp.patch,
> 0009-making-SinkManager-a-singleton.patch,
> 0010-making-DefsTables-a-singleton.patch,
> 0011-making-StageManager-a-singleton.patch,
> 0012-making-MessagingService-a-singleton.patch,
> 0013-making-QueryProcessor-a-singleton.patch,
> 0014-refactoring-static-methods-on-Tracing.patch,
> 0015-removing-static-state-from-BatchlogManager.patch,
> 0016-removing-static-method-from-CommitLog.patch,
> 0017-OutboundTcpConnection-removing-singleton-access-from.patch,
> 0018-FBUtilities-removing-getLocalAddress-getBroadcastAdd.patch,
> 0019-PendingRangeCalculatorService-removing-singleton-acc.patch,
> 0020-ActiveRepairService-removing-static-members-and-meth.patch,
> 0021-RowDataResolver-removing-static-singleton-access-fro.patch,
> 0022-AbstractReadExecutor-removing-static-method.patch,
> 0023-StorageServiceAccessor-removing-static-singleton-acc.patch,
> 0024-FileUtils-removing-static-singleton-accesses-from-st.patch,
> 0025-ResourceWatcher-removing-static-singleton-access-fro.patch,
> 0026-TokenMetadata-removing-static-singleton-access-from-.patch,
> 0027-OutboundTcpConnectionPool-removing-static-singleton-.patch,
> 0028-Cassandra-PasswordAuthenticator-making-static-method.patch,
> 0029-CompactionMetrics-making-static-method-instance-meth.patch,
> 0030-ClientState-splitting-configured-QueryHandler-instan.patch,
> 0031-SSTableReader-splitting-static-factory-methods-into-.patch,
> 0032-Keyspace-splitting-static-factory-methods-and-state-.patch,
> 0033-ColumnFamilyStore-splitting-static-factory-methods-a.patch,
> 0034-TriggerDefinition-removing-static-singleton-access-f.patch,
> 0035-CFMetaData-splitting-off-static-factory-methods-onto.patch,
> 0036-KSMetaData-splitting-off-static-factory-methods-onto.patch,
> 0037-SystemKeyspace-moving-system-keyspace-definitions-on.patch,
> 0038-UTMetaData-refactoring-static-singleton-accesses-for.patch,
> 0039-CounterId-removing-static-singleton-accesses-from-st.patch,
> 0040-AtomicBtreeColumns-replacing-SystemKeyspace-CFMetaDa.patch
>
>
> 1st step of CASSANDRA-7837.
> Things like DatabaseDescriptor.getPartitioner() should become
> DatabaseDescriptor.instance.getPartitioner(). In cases where there is a mix
> of instance state and static functionality (Keyspace & ColumnFamilyStore
> classes), the static portion should be split off into singleton factory
> classes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
