PROPERTY_FILE_PREFIX ?

On Thu, Oct 13, 2016 at 3:05 PM, John Blum <jb...@pivotal.io> wrote:

> -1 for introducing this change as well.
>
> Also -1 for introducing any additional constants/workarounds.  Either fix
> it the way it should be fixed or do nothing at all.
>
> On Thu, Oct 13, 2016 at 2:45 PM, Kirk Lund <kl...@pivotal.io> wrote:
>
> > -1 at this point I'm against making this change for 1.0.0
> >
> > I'll still work towards fixing GEODE-1466 properly but it'll be fixed on
> > develop within the next week or so.
> >
> > -Kirk
> >
> >
> > On Thu, Oct 13, 2016 at 2:26 PM, Udo Kohlmeyer <ukohlme...@pivotal.io>
> > wrote:
> >
> > > If such a change is to be introduced.. maybe we call it `SYSTEM_PREFIX`
> > or
> > > something more generic that we could use within the Geode.
> > >
> > > Then we could hopefully cover many to most `gemfire` vs `geode`
> renaming.
> > >
> > > But I agree with @Anthony, if we aren't 100% certain about a change
> then
> > > we should hold off until the next release. There is always tomorrow. :D
> > >
> > > --Udo
> > >
> > >
> > >
> > > On 14/10/16 8:05 am, Swapnil Bawaskar wrote:
> > >
> > >> How about introducing a new GEMFIRE_FILE_PREFIX attribute that will
> > >> default
> > >> to "geode" while leaving GEMFIRE_PREFIX default to "gemfire"? Is this
> > >> something that will work?
> > >>
> > >> On Thu, Oct 13, 2016 at 1:48 PM, Anthony Baker <aba...@pivotal.io>
> > wrote:
> > >>
> > >> Hmmm, you would think it would be easier to change a file name :-)
> > >>>
> > >>> I don’t think we should be pushing destabilizing changes into a
> release
> > >>> branch.  If the changes aren’t ready now we always pick them up for
> the
> > >>> next release.
> > >>>
> > >>> Anthony
> > >>>
> > >>> On Oct 13, 2016, at 1:13 PM, Kirk Lund <kl...@apache.org> wrote:
> > >>>>
> > >>>> I'm currently working with Jared and we have spent a few days
> working
> > >>>> on GEODE-1466. We've been trying to get geode to the point where it
> > can
> > >>>> automatically search for, find and use either geode.properties or
> > >>>> gemfire.properties (preferring geode.properties if both are found).
> > >>>>
> > >>>> We were intending to break this up into three subtasks with the hope
> > of
> > >>>> including #1 in Geode 1.0.0 incubating:
> > >>>>
> > >>>> 1) locating geode.properties and gemfire.properties if user has not
> > >>>> specified a specific properties file
> > >>>>
> > >>>> 2) support specifying geode configuration properties with system
> > >>>>
> > >>> properties
> > >>>
> > >>>> geode.<property-name> as well as gemfire.<property-name>
> > >>>>
> > >>>> ex: -Dgemfire.off-heap-memory-size=40g or
> > -Dgeode.off-heap-memory-size=
> > >>>>
> > >>> 40g
> > >>>
> > >>>> 3) modify all other system properties in geode to support alias of
> > >>>>
> > >>> gemfire
> > >>>
> > >>>> as well as geode where the name of the system property currently
> > >>>> contains
> > >>>> gemfire
> > >>>>
> > >>>> ex: -Dgemfire.Query.VERBOSE=true or -Dgeode.Query.VERBOSE=true
> > >>>>
> > >>>> The community is planning to create the Geode 1.0.0 incubating RC
> > >>>>
> > >>> tomorrow.
> > >>>
> > >>>> The work we have completed so far involves modifying geode to search
> > for
> > >>>> both geode.properties and gemfire.properties to use whichever one is
> > >>>>
> > >>> found.
> > >>>
> > >>>> This turns out to be too complex to complete by tomorrow (send me a
> > >>>> email
> > >>>> directly if you want more detailed info which mostly involves
> > >>>> DistributionConfig and ConfigSource).
> > >>>>
> > >>>> In order to complete this in time, we need to use a different
> > strategy.
> > >>>> Instead of looking for geode.properties first and then
> > >>>>
> > >>> gemfire.properties,
> > >>>
> > >>>> we are proposing the following change to DistributionConfig:
> > >>>>
> > >>>> Change the GEMFIRE_PREFIX = "gemfire." constant to be configurable
> by
> > a
> > >>>> system property and change the default to be "geode." This places a
> > >>>>
> > >>> greater
> > >>>
> > >>>> burden on the user who is migrating from GemFire to Geode but
> doesn't
> > >>>>
> > >>> want
> > >>>
> > >>>> to rename gemfire.properties or gemfire system properties. By
> default,
> > >>>> Geode would search for geode.properties unless the user specifies a
> > new
> > >>>> system property with a different prefix ("gemfire."):
> > >>>>
> > >>>> String GEMFIRE_PREFIX = PropertiesPrefix.getGemfireOrGeodePrefix();
> > >>>>
> > >>>> Example of overriding this to be "gemfire.":
> > >>>>
> > >>>> -DgeodePropertiesPrefix="gemfire."
> > >>>> or
> > >>>> -DgeodePropertiesPrefix="gemfire"
> > >>>>
> > >>>> (we'll add the "." for you if you leave it out)
> > >>>>
> > >>>> Pivotal or other vendors could also alter this prefix as they see
> fit.
> > >>>>
> > >>>> There are 453 lines of production code that refer to this
> > GEMFIRE_PREFIX
> > >>>> constant. For example, every system property that contains
> "gemfire."
> > is
> > >>>> currently referring to the constant, so they would also be altered
> to
> > be
> > >>>> "geode." by default. The JMX notifications also refer to
> > GEMFIRE_PREFIX
> > >>>> such as: GEMFIRE_PREFIX + "distributedsystem.cache.client.joined".
> > >>>>
> > >>>> Does anyone know if anything referring to GEMFIRE_PREFIX is
> persisted
> > in
> > >>>> some way that would break if we make this change? For example, if we
> > >>>> persist any strings built with this constant to a diskstore, then
> > >>>>
> > >>> recovery
> > >>>
> > >>>> from that diskstore would be broken if the prefix value is "geode."
> > >>>>
> > >>> during
> > >>>
> > >>>> recovery of an old diskstore.
> > >>>>
> > >>>> Also, a user could conceivably override the GEMFIRE_PREFIX in some
> > >>>>
> > >>> members
> > >>>
> > >>>> of a cluster but not others which could break things in unexpected
> > ways.
> > >>>>
> > >>>> One more note: While reviewing uses of GEMFIRE_PREFIX we noticed
> that
> > >>>> AgentUtil supports "GEMFIRE" (hardcoded) and GEMFIRE_PREFIX+".home"
> > env
> > >>>> vars while all of the online docs specify setting GEMFIRE_HOME as an
> > env
> > >>>> var. I suspect this is already broken (I will file a ticket), but I
> > >>>> think
> > >>>> we will also be at risk of breaking additional things that may or
> may
> > >>>> not
> > >>>> be immediately detected by precheckin tests. It's also used by
> > >>>>
> > >>> DtdResolver
> > >>>
> > >>>> for the name of a dtd: new File(GEMFIRE_PREFIX + "dtd). We'll
> continue
> > >>>> looking for unusual or risky uses of the constant.
> > >>>>
> > >>>> Bottom line is making this change is higher risk than not making the
> > >>>> change, and there could be some fallout bugs that require fixes and
> > >>>> additional release candidates for 1.0.0.
> > >>>>
> > >>>> Does the community feel this change is desirable for 1.0.0? Or is it
> > >>>>
> > >>> better
> > >>>
> > >>>> to leave it as "gemfire." and move GEODE-1466 to post-1.0.0?
> > >>>>
> > >>>> Thanks,
> > >>>> Kirk
> > >>>>
> > >>>
> > >>>
> > >
> >
>
>
>
> --
> -John
> 503-504-8657
> john.blum10101 (skype)
>

Reply via email to