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 <[email protected]> wrote: > PROPERTY_FILE_PREFIX ? > > On Thu, Oct 13, 2016 at 3:05 PM, John Blum <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > 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 <[email protected]> > > > 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 <[email protected]> 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) > > >
