Yeah, introducing a new more focused prefix constant just for properties
would be better.

Sounds like consensus so far though is to just hold off for a fix that
refactors DistributionConfigImpl enough to make a good clean change to look
for both properties files.

-Kirk


On Thu, Oct 13, 2016 at 4:29 PM, Dave Barnes <dbar...@pivotal.io> wrote:

> 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