Github user mjwall commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/256#discussion_r116046884
  
    --- Diff: 
core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/lru/LruBlockCache.java
 ---
    @@ -99,116 +91,57 @@
       private final ScheduledExecutorService scheduleThreadPool = 
Executors.newScheduledThreadPool(1, new 
NamingThreadFactory("LRUBlockCacheStats"));
     
       /** Current size of cache */
    -  private final AtomicLong size;
    +  private AtomicLong size;
     
       /** Current number of cached elements */
    -  private final AtomicLong elements;
    +  private AtomicLong elements;
     
       /** Cache access count (sequential ID) */
    -  private final AtomicLong count;
    +  private AtomicLong count;
     
       /** Cache statistics */
    -  private final CacheStats stats;
    -
    -  /** Maximum allowable size of cache (block put if size > max, evict) */
    -  private long maxSize;
    -
    -  /** Approximate block size */
    -  private long blockSize;
    -
    -  /** Acceptable size of cache (no evictions if size < acceptable) */
    -  private float acceptableFactor;
    -
    -  /** Minimum threshold of cache (when evicting, evict until size < min) */
    -  private float minFactor;
    -
    -  /** Single access bucket size */
    -  private float singleFactor;
    -
    -  /** Multiple access bucket size */
    -  private float multiFactor;
    -
    -  /** In-memory bucket size */
    -  private float memoryFactor;
    +  private CacheStats stats;
     
       /** Overhead of the structure itself */
    -  private long overhead;
    +  private final long overhead;
    +
    +  private final LruBlockCacheConfiguration conf;
     
       /**
        * Default constructor. Specify maximum size and expected average block 
size (approximation is fine).
        *
        * <p>
        * All other factors will be calculated based on defaults specified in 
this class.
        *
    +   * @param conf
    +   *          block cache configuration
        * @param maxSize
        *          maximum size of cache, in bytes
        * @param blockSize
        *          approximate size of each block, in bytes
        */
    -  public LruBlockCache(long maxSize, long blockSize) {
    -    this(maxSize, blockSize, true);
    -  }
    +  public LruBlockCache(final LruBlockCacheConfiguration conf) {
    +    this.conf = conf;
     
    -  /**
    -   * Constructor used for testing. Allows disabling of the eviction thread.
    -   */
    -  public LruBlockCache(long maxSize, long blockSize, boolean 
evictionThread) {
    -    this(maxSize, blockSize, evictionThread, (int) Math.ceil(1.2 * maxSize 
/ blockSize), DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL, 
DEFAULT_MIN_FACTOR,
    -        DEFAULT_ACCEPTABLE_FACTOR, DEFAULT_SINGLE_FACTOR, 
DEFAULT_MULTI_FACTOR, DEFAULT_MEMORY_FACTOR);
    -  }
    +    int mapInitialSize = (int) Math.ceil(1.2 * conf.getMaxSize() / 
conf.getBlockSize());
     
    -  /**
    -   * Configurable constructor. Use this constructor if not using defaults.
    -   *
    -   * @param maxSize
    -   *          maximum size of this cache, in bytes
    -   * @param blockSize
    -   *          expected average size of blocks, in bytes
    -   * @param evictionThread
    -   *          whether to run evictions in a bg thread or not
    -   * @param mapInitialSize
    -   *          initial size of backing ConcurrentHashMap
    -   * @param mapLoadFactor
    -   *          initial load factor of backing ConcurrentHashMap
    -   * @param mapConcurrencyLevel
    -   *          initial concurrency factor for backing CHM
    -   * @param minFactor
    -   *          percentage of total size that eviction will evict until
    -   * @param acceptableFactor
    -   *          percentage of total size that triggers eviction
    -   * @param singleFactor
    -   *          percentage of total size for single-access blocks
    -   * @param multiFactor
    -   *          percentage of total size for multiple-access blocks
    -   * @param memoryFactor
    -   *          percentage of total size for in-memory blocks
    -   */
    -  public LruBlockCache(long maxSize, long blockSize, boolean 
evictionThread, int mapInitialSize, float mapLoadFactor, int 
mapConcurrencyLevel, float minFactor,
    -      float acceptableFactor, float singleFactor, float multiFactor, float 
memoryFactor) {
    -    if (singleFactor + multiFactor + memoryFactor != 1) {
    +    if (conf.getSingleFactor() + conf.getMultiFactor() + 
conf.getMemoryFactor() != 1) {
    --- End diff --
    
    consider moving these checks the constructor of the 
LruBlockCacheConfiguration object so you can not make an invalid conf.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to