Improving the existing code is a great goal. While cleaning up code is nice KahaDB has gotten pretty stable over the > years and doing a bunch of refactoring just opens it up to new bugs that > have to be fixed. Fixing bugs is not a problem however I tend to be more > sensitive to store related changes because of the possible data loss or > corruption issues to production data that can occur from store bugs vs some > other random bug in the broker. >
I understand the desire to avoid the risk of introducing bugs. However, as long as the project is active and maintained, that really isn't a valid approach to its maintenance overall. So that leads us to the question of risk mitigation. Build-time tests are an industry standard, and ActiveMQ certainly has a large number of such tests. Code reviews are another best-practice, and there are multiple individuals looking at these code changes now. More are always welcome, and access is certainly not a problem! If there are specific concerns within the code changes, let's discuss them. It'll be great to have actual technical discussions! As for the concern that this is the broker's storage, similar arguments could be made for just about any part of the code. Reliable handling of messages is ActiveMQ's core benefit to its users. > FWIW, the current community goal is for ActiveMQ Artemis to become ActiveMQ 6.x when acceptable feature parity is reached (which is actively being > worked on). > Whether Artemis will eventually become ActiveMQ 6.x is not pertitent to this discussion. Let's not open that discussion as part of this technical code conversation. Making the existing code base, which has heavy usage in the industry, more maintainable is always a good goal to achieve. And having more folks in the community engaging in working on the project can only benefit the entire community regardless of the long-term destination. Art On Mon, Nov 26, 2018 at 10:22 AM Justin Bertram <jbert...@apache.org> wrote: > FWIW, the current community goal is for ActiveMQ Artemis to become ActiveMQ > 6.x when acceptable feature parity is reached (which is actively being > worked on). > > > Justin > > > On Mon, Nov 26, 2018 at 11:01 AM Jamie G. <jamie.goody...@gmail.com> > wrote: > > > The idea here is to ensure that it’s development and maintenance > > moving forward is easier as we work to make it better in the future. > > > > KahaDB currently has massive classes (KahaDBStore, MessageDatabase) > > and code base and is extremely hard to follow. My desire to do this > > was to make this so we could make the continued maintenance easier and > > also make it more inviting to improvements. The unit tests all pass, > > so as long as we have a good solid testing coverage, the risks are not > > huge. If a bug appears to be introduced, than we may have uncovered a > > testing hole - finding these is a good thing. > > > > Since we are going to continue to support ActiveMQ moving forward, > > it’s a good idea to make it more maintainable. My motivation to doing > > this was spurred by the recent JIRAs surrounding KahaDB I helped out > > upon. I really believe this is a good improvement to help moving > > ActiveMQ forward. > > On Mon, Nov 26, 2018 at 12:33 PM Christopher Shannon > > <christopher.l.shan...@gmail.com> wrote: > > > > > > I'm not really sure this is worthwhile or something we want to do...I > > would > > > have to think about it more before I gave it a +1. > > > > > > While cleaning up code is nice KahaDB has gotten pretty stable over the > > > years and doing a bunch of refactoring just opens it up to new bugs > that > > > have to be fixed. Fixing bugs is not a problem however I tend to be > more > > > sensitive to store related changes because of the possible data loss or > > > corruption issues to production data that can occur from store bugs vs > > some > > > other random bug in the broker. > > > > > > On Sun, Nov 25, 2018 at 11:59 PM Jean-Baptiste Onofré <j...@nanthrax.net > > > > > wrote: > > > > > > > OK, got it. It's more a syntax/codebase organization refactoring. > > > > > > > > If there's no impact on the behavior and features, +1 from my side. > > > > > > > > Regards > > > > JB > > > > > > > > On 25/11/2018 21:21, Jamie G. wrote: > > > > > Initially its to make KahaDB classes easier to read & maintain. > > > > > Eventually it will help in features/performance; smaller classes > are > > > > > easier to grok, easier to see improvements. > > > > > > > > > > Instead of trying to refactor all of it in one go, I'm taking the > > > > > approach of one area at a time. > > > > > > > > > > One pass for breaking out objects. > > > > > Another pass for small functional improvements. > > > > > Perhaps future passes for new Java features (bring all code up to > > Java > > > > > 8 perhaps?). > > > > > > > > > > On Sun, Nov 25, 2018 at 4:32 PM Jean-Baptiste Onofré < > > j...@nanthrax.net> > > > > wrote: > > > > >> > > > > >> Hi Jamie, > > > > >> > > > > >> That's interesting. > > > > >> > > > > >> What's the rationale behind the refactoring ? New features or perf > > > > >> improvements ? > > > > >> > > > > >> Regards > > > > >> JB > > > > >> > > > > >> On 25/11/2018 20:16, Jamie G. wrote: > > > > >>> Hi All, > > > > >>> > > > > >>> I've taken some time to prototype a refactored KahaDBStore class: > > > > >>> https://github.com/jgoodyear/activemq/tree/KahaDBRefactor > > > > >>> > > > > >>> As KahaDBStore exists in Master, it contains 7 internal classes, > > over > > > > >>> some 1677 lines of code. > > > > >>> > > > > >>> In my refactor branch I've separated out those classes into their > > own > > > > >>> files, and applied some gentle clean code practices to help make > > these > > > > >>> files easier to read and maintain. > > > > >>> > > > > >>> I'd like to gather feed back from the community; I've taken care > to > > > > >>> change functionality as little as possible - the aim here is to > > reduce > > > > >>> complexity and improve maintainability. If the community feels > > this is > > > > >>> a worth while goal than I'll open a card on Jira & prepare a PR. > > > > >>> > > > > >>> Notes: > > > > >>> ActiveMQ KahaDB Store and ActiveMQ-Unit-Tests suites remain > > passing > > > > >>> after refactor. > > > > >>> > > > > >>> Cheers, > > > > >>> Jamie > > > > >>> > > > > >> > > > > >> -- > > > > >> Jean-Baptiste Onofré > > > > >> jbono...@apache.org > > > > >> http://blog.nanthrax.net > > > > >> Talend - http://www.talend.com > > > > > > > > -- > > > > Jean-Baptiste Onofré > > > > jbono...@apache.org > > > > http://blog.nanthrax.net > > > > Talend - http://www.talend.com > > > > > > >
