[ 
https://issues.apache.org/jira/browse/METRON-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15712230#comment-15712230
 ] 

ASF GitHub Bot commented on METRON-601:
---------------------------------------

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

    https://github.com/apache/incubator-metron/pull/381#discussion_r90467298
  
    --- Diff: 
metron-platform/metron-test-utilities/src/main/java/org/apache/metron/test/mock/MockHTable.java
 ---
    @@ -437,15 +463,14 @@ public ResultScanner getScanner(byte[] family, byte[] 
qualifier)
         return getScanner(scan);
       }
     
    -  List<Put> putLog = new ArrayList<>();
    --- End diff --
    
    Good that we're digging into this.  I'm not totally sure that this is 
thread-safe.  You may be right.  I have used `memoize` for a thread-safe, 
lazy-loading cache.  That's the use case I've seen others use it for.  But 
maybe with that hammer, everything is looking like a nail to me. :)
    
    But in my opinion if `memoize` is not thread-safe, then `synchronizedList` 
is not either.  See the code snippets below.  What `synchronizedList` is doing 
seems to be a subset of what `memoize` does.  No?
    
    ```
        MemoizingSupplier(Supplier<T> delegate) {
          this.delegate = delegate;
        }
    
        public T get() {
          if(!this.initialized) {
            synchronized(this) {
              if(!this.initialized) {
                Object t = this.delegate.get();
                this.value = t;
                this.initialized = true;
                return t;
              }
            }
          }
    
          return this.value;
        }
      }
    ```
    
    versus
    
    ```
        ThreadSafeSupplier(Supplier<T> delegate) {
          this.delegate = delegate;
        }
    
        public T get() {
          Supplier var1 = this.delegate;
          synchronized(this.delegate) {
            return this.delegate.get();
          }
        }
      }
    ```


> MockHTable Put Log is Not Thread Safe
> -------------------------------------
>
>                 Key: METRON-601
>                 URL: https://issues.apache.org/jira/browse/METRON-601
>             Project: Metron
>          Issue Type: Sub-task
>            Reporter: Nick Allen
>            Assignee: Nick Allen
>
> The MockHTable uses an ArrayList to store a log of Puts that have been 
> submitted against the MockHTable.  The MockHTable, along with the put log, is 
> accessed from multiple threads during the integration tests.  Access to the 
> Put log is not thread safe, which is likely at least one cause of METRON-597.
> The Put log is used by multiple tests, but more so by the 
> ProfilerIntegrationTest.  This tests polls the list to block the thread until 
> the expected number of Puts have been submitted.  This is likely why this 
> test is more impacted by this issue than others.
> The Put Log needs to made thread safe.  See 
> `org.apache.metron.test.mock.MockHTable.putLog`



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to