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) >