geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kirk Lund <kirk.l...@gmail.com>
Subject Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config
Date Tue, 13 Jun 2017 16:30:42 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message