> On June 12, 2017, 7:59 p.m., Jared Stewart wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java > > Lines 327 (patched) > > <https://reviews.apache.org/r/60010/diff/1/?file=1748827#file1748827line329> > > > > Are you sure that we want to keep this response around as a member > > variable? I'm afraid that it might be stale (in the sense that the cluster > > config has since been changed) if this response is referred to later on.
Please see the new diff. I changed it from final to volatile and null it out when the GemFireCacheImpl is finished using it. The only other way to handle it would be as a ThreadLocal instead of a member variable. The advantage of a ThreadLocal is that it wouldn't permanently use an object header worth of heap memory after initialization completes. - Kirk ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60010/#review177668 ----------------------------------------------------------- On June 13, 2017, 4:29 p.m., Kirk Lund wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60010/ > ----------------------------------------------------------- > > (Updated June 13, 2017, 4:29 p.m.) > > > Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, > and Patrick Rhomberg. > > > Bugs: GEODE-3062 > https://issues.apache.org/jira/browse/GEODE-3062 > > > Repository: geode > > > Description > ------- > > Add new test to ClusterConfigWithSecurityDUnitTest that fails due to bug > GEODE-3062. > > Remove unused Cache param from applyClusterPropertiesConfiguration so it can > be called during Cache construction. > > Move cluster config request to Cache construction and handle jars and > properties there. Create new SecurityService in constructor and overwrite the > SecurityService in InternalDistributedSystem. > > NOTE: We will later have to fix GEODE-3061 by moving cluster config request > from Cache to InternalDistributedSystem construction so that IDS can be > created with gemfire.properties from cluster config. At that time we would > rip out both cluster config request and creation of security service from > Cache construction and pass both into Cache construction. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java > 22edb6f06c7791929cc9a033ca1a1bfed5751a47 > > geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java > 4f4881fe39116faa505bc2fbec74efd669efe0ef > > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java > 40df0c7dcac8827a381c268c1f90e6acfb97a7f1 > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java > c551ca9104a85dcf54c0bebbc4178fce4114a416 > > > Diff: https://reviews.apache.org/r/60010/diff/2/ > > > Testing > ------- > > Precheckin passes > > > Thanks, > > Kirk Lund > >
