You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jason Huynh <jh...@pivotal.io> on 2017/03/10 00:26:45 UTC

Review Request 57483: GEODE-2643:Combine chunk and file region into a single region

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57483/
-----------------------------------------------------------

Review request for geode, Barry Oglesby, Lynn Hughes-Godfrey, nabarun nag, Dan Smith, and xiaojian zhou.


Repository: geode


Description
-------

* removed file and chunk count from stat (no longer able to use localSize and not sure how helpful those stats really were)
* removed tests that were doing checks against bucketRegions
* removed a test that was hard to maintain in FileSystemJUnitTest (it required enough operations to 
* todo before checkin: rename fileBucket variables to fileAndChunkBucket OR rename fileAndChunkRegion to fileRegion.  The Suffix of .files is still being used.


Diffs
-----

  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java 7e685b7 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneBucketListener.java 9532249 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 4aa24b5 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java 18428ec 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java 660816d 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemStats.java 85ae6d7 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneIndexDestroyDUnitTest.java 6260075 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java 5ac01b8 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java 5fe2df5 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java 93cc0a8 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexRecoveryHAIntegrationTest.java b50db98 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PartitionedRepositoryManagerJUnitTest.java 9c603c7 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectoryJUnitTest.java 32249e4 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/distributed/DistributedScoringJUnitTest.java 6062904 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java ee41e40 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java 42cc2bc 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java e3e2787 
  geode-lucene/src/test/java/org/apache/geode/cache/lucene/test/LuceneTestUtilities.java 329dee9 


Diff: https://reviews.apache.org/r/57483/diff/1/


Testing
-------

geode-lucene:precheckin


Thanks,

Jason Huynh


Re: Review Request 57483: GEODE-2643:Combine chunk and file region into a single region

Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57483/#review168661
-----------------------------------------------------------




geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
Line 85 (original), 82 (patched)
<https://reviews.apache.org/r/57483/#comment240912>

    mix using "fileRegion" and "fileAndChunkRegion" in many places.



geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java
Line 58 (original), 55 (patched)
<https://reviews.apache.org/r/57483/#comment240910>

    fileRegion is still Map<String, File>? But in line 41, it is "Map fileAndChunkRegion".



geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java
Line 184 (original), 180 (patched)
<https://reviews.apache.org/r/57483/#comment240918>

    Looks like fileAndChunkRegion is not <String, File>. So where is the chunk? Where is the "File"?


- xiaojian zhou


On March 10, 2017, 12:26 a.m., Jason Huynh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57483/
> -----------------------------------------------------------
> 
> (Updated March 10, 2017, 12:26 a.m.)
> 
> 
> Review request for geode, Barry Oglesby, Lynn Hughes-Godfrey, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * removed file and chunk count from stat (no longer able to use localSize and not sure how helpful those stats really were)
> * removed tests that were doing checks against bucketRegions
> * removed a test that was hard to maintain in FileSystemJUnitTest (it required enough operations to 
> * todo before checkin: rename fileBucket variables to fileAndChunkBucket OR rename fileAndChunkRegion to fileRegion.  The Suffix of .files is still being used.
> 
> 
> Diffs
> -----
> 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java 7e685b7 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneBucketListener.java 9532249 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 4aa24b5 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java 18428ec 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java 660816d 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemStats.java 85ae6d7 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneIndexDestroyDUnitTest.java 6260075 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java 5ac01b8 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java 5fe2df5 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java 93cc0a8 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexRecoveryHAIntegrationTest.java b50db98 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PartitionedRepositoryManagerJUnitTest.java 9c603c7 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectoryJUnitTest.java 32249e4 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/distributed/DistributedScoringJUnitTest.java 6062904 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java ee41e40 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java 42cc2bc 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java e3e2787 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/test/LuceneTestUtilities.java 329dee9 
> 
> 
> Diff: https://reviews.apache.org/r/57483/diff/1/
> 
> 
> Testing
> -------
> 
> geode-lucene:precheckin
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>


Re: Review Request 57483: GEODE-2643:Combine chunk and file region into a single region

Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57483/#review168575
-----------------------------------------------------------


Fix it, then Ship it!





geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java
Lines 62 (patched)
<https://reviews.apache.org/r/57483/#comment240825>

    toList is probably more efficient than toSet here, so it doesn't have to hash all of your strings.


- Dan Smith


On March 10, 2017, 12:26 a.m., Jason Huynh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57483/
> -----------------------------------------------------------
> 
> (Updated March 10, 2017, 12:26 a.m.)
> 
> 
> Review request for geode, Barry Oglesby, Lynn Hughes-Godfrey, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * removed file and chunk count from stat (no longer able to use localSize and not sure how helpful those stats really were)
> * removed tests that were doing checks against bucketRegions
> * removed a test that was hard to maintain in FileSystemJUnitTest (it required enough operations to 
> * todo before checkin: rename fileBucket variables to fileAndChunkBucket OR rename fileAndChunkRegion to fileRegion.  The Suffix of .files is still being used.
> 
> 
> Diffs
> -----
> 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java 7e685b7 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneBucketListener.java 9532249 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 4aa24b5 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java 18428ec 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java 660816d 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemStats.java 85ae6d7 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneIndexDestroyDUnitTest.java 6260075 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java 5ac01b8 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java 5fe2df5 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java 93cc0a8 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexRecoveryHAIntegrationTest.java b50db98 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PartitionedRepositoryManagerJUnitTest.java 9c603c7 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectoryJUnitTest.java 32249e4 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/distributed/DistributedScoringJUnitTest.java 6062904 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java ee41e40 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java 42cc2bc 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java e3e2787 
>   geode-lucene/src/test/java/org/apache/geode/cache/lucene/test/LuceneTestUtilities.java 329dee9 
> 
> 
> Diff: https://reviews.apache.org/r/57483/diff/1/
> 
> 
> Testing
> -------
> 
> geode-lucene:precheckin
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>