No actual removal would be done. All would stay as it is, it would just call SnapshotMangers's stuff where appropriate. SnapshotManager(MBean) would be introduced to centralize snapshots' logic. By deprecation, I meant that we would put "@Deprecated" annotation on StorageServiceMBean methods as it is done on a lot of other methods already. Deprecated annotation has a field called "forRemoval" which is set to "false" by default. It means that while we marked such a method as deprecated, we do not plan whatsoever to remove it in the future. We just signal to programmers / users that there is another way of doing it which is preferable.
If you do not even want to mark it as Deprecated (as it has been done countless times already), we might go with that too. The "deprecation" here is more about being semantic rather than something users need to deal with now or in the future. They don't. I think that breaking the "monolithic" services such as StorageService which counts 5600 lines now is a good idea. That class is notoriously known to be too big and complex. There is stuff which does not even have anything in common with "storage". We just want to simplify that by extracting common family of logic (e.g. snapshots) into a dedicated class, remove the duplicities etc, however, because we deal here with a lot of legacy, the actual methods in StorageService(MBean) would stay, it would be just delegated away. On Thu, Aug 1, 2024 at 4:22 PM Jon Haddad <j...@jonhaddad.com> wrote: > I would prefer we kept the old methods in place without deprecation. > > * I (and I assume others) learn about what I can do with JMX by using > JConsole and browsing visually, and find it helpful to be able to look at a > table and understand what I can do with it. Approaching it the other way > isn't particularly intuitive to me, so from a user perspective, it's less > friendly to at least a subset of our users. > > * Unless I misunderstand the work involved, I think we're only talking > about calling another method. The overhead on our side would be trivial in > that case. Deprecation of anything is disruptive, and the gain sounds > trivial at best. I don't think we've deprecated or removed anything that > mentions "column family" or "cf" even though that hasn't been our > user-facing terminology in close to a decade. Please correct me if I'm > wrong on any of this. > > Thanks, > Jon > > > On Thu, Aug 1, 2024 at 3:03 AM Štefan Miklošovič <smikloso...@apache.org> > wrote: > >> We are refactoring the snapshotting subsystem (CASSANDRA-18111) by >> consolidating all snapshot logic in Cassandra code into a dedicated >> SnapshotManager class where everything is centralized and dandy. >> >> The current state of snapshotting code is that over the years it became >> scattered all over the place, hard to get the whole picture, hard to >> maintain ... probable / possible concurrency issues etc. By doing 18111 we >> will also save some disk IO. >> >> While doing so, we are indeed keeping everything functionally-wise as it >> was before etc. but we think that the introduction of a dedicated >> SnapshotManagerMBean (like this (1)) would be a good idea. We would keep >> stuff in StorageServiceMBean / CFS there, it would just call >> SnapshotManager methods and we would deprecate StorageServiceMBean methods. >> >> Since the introduction of SnapshotManagerMBean introduces a public-facing >> API, I want to run this through ML to increase the awareness and get >> approval for doing so. >> >> Does this make sense to people? >> >> Regards >> >> (1) >> https://github.com/pauloricardomg/cassandra/blob/CASSANDRA-18111-review-2/src/java/org/apache/cassandra/service/snapshot/SnapshotManagerMBean.java >> >> >>
