ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Scott Gray <scott.g...@hotwaxsystems.com>
Subject Re: Help in reviewing my refactoring of CatalinaContainer
Date Tue, 20 Jun 2017 05:47:21 GMT
Hi Taher,

The jira summary of changes sounds like a re-organization without any
(intentional) changes in functionality.  Personally I would just go ahead
and commit it.

Regards
Scott

On 20 June 2017 at 09:13, Taher Alkhateeb <slidingfilaments@gmail.com>
wrote:

> Hey Folks. I know everyone's probably preoccupied. It would be great to get
> some feedback on OFBIZ-9392 though given how critical this piece of code
> is. I'd feel much better about committing after a few reviews.
>
> On Thu, Jun 15, 2017 at 9:22 PM, Taher Alkhateeb <
> slidingfilaments@gmail.com
> > wrote:
>
> > Hi Michael,
> >
> > I think most of our documentation is outdated. So your best bet is [1]. I
> > look forward to your feedback.
> >
> > @Scott you seem to have good experience with clustering, It would be
> > really great if we can have your eyes on the code or some testing of the
> > functionality.
> >
> > [1] https://tomcat.apache.org/tomcat-8.0-doc/cluster-howto.html
> >
> > Cheers,
> > --
> > Taher Alkhateeb
> >
> > On Wed, Jun 14, 2017 at 11:07 PM, Michael Brohl <
> michael.brohl@ecomify.de>
> > wrote:
> >
> >> Hi Taher,
> >>
> >> Am 14.06.17 um 19:56 schrieb Taher Alkhateeb:
> >>
> >>> Okay, after a little bit of investigation I think I got what caused the
> >>> crashes on my machine:
> >>> - I removed "cluster.registerManager(manager);" from the code base
> >>> because
> >>> it was commented out in the original code base
> >>>
> >>
> >> That explains why I didn't get these errors with the pre-refactoring
> >> setup :-)
> >>
> >> - I commented out "<property name="mcast-bind-addr"
> value="192.168.2.1"/>"
> >>> from ofbiz-component.xml because it was conflicting with my local LAN
> >>> setup.
> >>>
> >>
> >> I did the same with the current trunk and now OFBiz is starting with
> >> enabled clustering.
> >>
> >> After doing the above and testing the cluster, everything worked. Bug
> >>> again, given that I'm treading in new territory, appreciate all the
> help.
> >>>
> >>
> >> Do we have some documentation on how to setup the cluster environment
> for
> >> testing, to have some kind of reference setup?
> >> It would help to make testing easier.
> >>
> >> I will try to review your patch in the next days.
> >>
> >> Regards,
> >> Michael
> >>
> >>
> >> The new patch is available at
> >>> https://issues.apache.org/jira/browse/OFBIZ-9392
> >>>
> >>> Cheers,
> >>>
> >>> Taher Alkhateeb
> >>>
> >>> On Tue, Jun 13, 2017 at 9:17 AM, Taher Alkhateeb <
> >>> slidingfilaments@gmail.com
> >>>
> >>>> wrote:
> >>>> So the good thing is that the refactoring exercise surfaced this
> issue.
> >>>> Based on feedback from Scott and Michael I suspect that the issue
> might
> >>>> be
> >>>> perhaps minor such as a missing configuration or dependency that's
> >>>> causing
> >>>> all these errors.
> >>>>
> >>>> I'll try to dig out the root cause and your help would be great to try
> >>>> and
> >>>> zoom in on the problem preferably on the refactored code, but even
> >>>> finding
> >>>> the root cause on existing code would greatly help.
> >>>>
> >>>> Cheers,
> >>>>
> >>>> Taher
> >>>>
> >>>> On Jun 13, 2017 4:16 AM, "Scott Gray" <scott.gray@hotwaxsystems.com>
> >>>> wrote:
> >>>>
> >>>> The project I'm currently on is actively using clustering (session
> >>>>> replication) and I know for certain this isn't the only project
nor
> >>>>> are we
> >>>>> the only vendor to use it.  I'm using an older version of OFBiz
> though
> >>>>> so
> >>>>> I
> >>>>> can't confirm if the latest codebase has it working.  It can be
> tricky
> >>>>> to
> >>>>> set up and does have a tendency to crash ofbiz during startup if
you
> >>>>> don't
> >>>>> have it configured correctly.
> >>>>>
> >>>>> Definitely a -1 for removal.
> >>>>>
> >>>>> Regards
> >>>>> Scott
> >>>>>
> >>>>> On 13 June 2017 at 08:57, Taher Alkhateeb <
> slidingfilaments@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>> Hi Everyone,
> >>>>>>
> >>>>>> In reference to [1] and the refactoring work, and after multiple
> >>>>>>
> >>>>> different
> >>>>>
> >>>>>> tests, I came to realize that Tomcat clustering implementation
is
> >>>>>> broken
> >>>>>> (both before and after the refactoring). The reasons are complex
but
> >>>>>>
> >>>>> they
> >>>>>
> >>>>>> essentially have to do with setting up the right parameters
for the
> >>>>>> cluster. So I suspect no one is using this feature?
> >>>>>>
> >>>>>> It is therefore, my recommendation to remove clustering from
> >>>>>> CatalinaContainer which would substantially reduce complexity
of
> both
> >>>>>>
> >>>>> the
> >>>>>
> >>>>>> component configuration file and the accompanying source code
which
> >>>>>>
> >>>>> would
> >>>>>
> >>>>>> shave off at least 120 lines of code.
> >>>>>>
> >>>>>> WDYT
> >>>>>>
> >>>>>> [1] https://issues.apache.org/jira/browse/OFBIZ-9392
> >>>>>>
> >>>>>> On Thu, Jun 8, 2017 at 3:38 PM, Taher Alkhateeb <
> >>>>>> slidingfilaments@gmail.com>
> >>>>>> wrote:
> >>>>>>
> >>>>>> Hey folks,
> >>>>>>>
> >>>>>>> It was very painful and slow, but I finally got a working
almost
> >>>>>>> full-rewrite of the CatalinaContainer. I need help with
testing,
> >>>>>>>
> >>>>>> reviews,
> >>>>>
> >>>>>> and code improvements.
> >>>>>>>
> >>>>>>> Details are found in [1]. There are no functional changes,
just a
> >>>>>>>
> >>>>>> rewrite.
> >>>>>>
> >>>>>>> I appreciate all the help and feedback.
> >>>>>>>
> >>>>>>> [1] https://issues.apache.org/jira/browse/OFBIZ-9392
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>>
> >>>>>>> Taher Alkhateeb
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>
> >>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message