wchevreuil commented on PR #7464: URL: https://github.com/apache/hbase/pull/7464#issuecomment-3611718902
> > The final version of this ReadOnlyController became too long and difficult to follow. I wonder if we should provide separate implementations for each of the observers/coprocessors being overridden here, and register those accordingly. WDYT? > > @wchevreuil I agree with @sharmaar12: > > > For making cluster read-only, earlier we have to add only one co-processor class in hbase-site.xml. For example, > > hbase.coprocessor.region.classes org.apache.hadoop.hbase.security.access.ReadOnlyController But if we split in 4 files then user may need to add all of them. Skipping one of it will make cluster inconsistent. So I feel having one is less error prone than 4. > > My understanding (like @sharmaar12 said) is if we split `ReadOnlyController` into multiple files, then each separate coprocessor would need to be added to the config. We already need to add the `ReadOnlyController` coprocessor three times - one for master, one for regionserver, and one for region. For example, an `hbase-site.xml` file needs to have the following: > > ``` > <property> > <name>hbase.coprocessor.master.classes</name> > <value>org.apache.hadoop.hbase.security.access.ReadOnlyController</value> > </property> > <property> > <name>hbase.coprocessor.regionserver.classes</name> > <value>org.apache.hadoop.hbase.security.access.ReadOnlyController</value> > </property> > <property> > <name>hbase.coprocessor.region.classes</name> > <value>org.apache.hadoop.hbase.security.access.ReadOnlyController</value> > </property> > ``` > > If `ReadOnlyController` is split into multiple coprocessor files, then wouldn't we need each of these new coprocessors separately for the master, regionserver, and region? I think that's even another argument for separation (the old design principle, separation of concerns). I agree that we should pursue configuration simplicity, so rather than requiring users to manually set coprocessors in hbase-site.xml, we should define a single property for marking a cluster as a read replica, then programatically register the related coprocessors during initialisation. See MasterCoprocessorHost and RegionServerCoprocessorHost, these are the components responsible for loading coprocessors on master and region server, respectively. -- 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]
