jihoonson commented on issue #7547: Add support minor compaction with segment 
locking
URL: https://github.com/apache/incubator-druid/pull/7547#issuecomment-511966275
 
 
   @clintropolis I ran some benchmark and here are some results. The benchmark 
code is available in [my 
branch](https://github.com/jihoonson/druid/blob/minor-compaction-benchmark/benchmarks/src/main/java/org/apache/druid/timeline/VersionedIntervalTimelineBenchmark.java)
 and will raise another pr for it after this PR is merged. Please note that 
this benchmark doesn't compare the performance of `VersionedIntervalTimeline` 
of this PR with that of the master branch. The primary purpose of this 
benchmark is to measure how slow `VersionedIntervalTimeline` is with 
segmentLock after this PR. (When timeChunk lock is used, 
`VersionedIntervalTimeline` performs a bit more operations such as creating an 
`OvershadowableManager` of a single partitionChunk than that of master. 
However, the overall performance wouldn't be much different.)
   
   TL;DR `VersionedIntervalTimeline` is slower with segment lock than that with 
timeChunk lock, but it's still acceptable. 
   
   ### Data setup
   
   A synthetic segments were created to emulate the usual compaction scenario 
where initial segments are created and then they got compacted while new 
segments are appended. The benchmark first generates 
`numInitialRootGenSegmentsPerInterval` segments per interval. Then, it 
generates `numInitialRootGenSegmentsPerInterval * 
COMPACTED_SEGMENTS_RATIO_TO_INITIAL_SEGMENTS` compacted segments which 
overwrites the segments of the previous generation. It also generates new 
appending segments. This can be repeated more than once based on 
`numNonRootGenerations`.
   
   Here, I pasted only some of results, e.g., when segmentGranularity = MONTH 
or # of non-root generations = 5, because they look similar. In every graph, 
blue and red represent throughput when using timeChunk lock and segment lock, 
respectively. Line charts show the throughput with varying # of total segments 
(including overshadowed ones). The _log scale_ is used for y axis of all line 
charts. The bar charts just show the one data point in line charts where # of 
segments = 160 which is the number Druid dataSources usually have per timeChunk.
   
   #### Lookup
   
   ![Lookup - varying # of 
segments](https://user-images.githubusercontent.com/2322288/61318527-28191980-a7ba-11e9-9a44-1028779af121.png)
   ![Lookup - 160 
segments](https://user-images.githubusercontent.com/2322288/61318529-294a4680-a7ba-11e9-8998-339754399671.png)
   
   To measure the throughput of the `lookup` method, the benchmark picks up a 
random interval which spans 3 time chunks based on segmentGranularity. This 
result says the timeline is pretty faster with timeChunk lock than using 
segment lock. But 23620.913 ops/s is still fast. Regarding peak CPU usage, 
there's no big difference between using timeChunk lock and segment lock.
   
   #### FindFullyOvershadowed
   
   ![findFullyOvershadowed - varying # of 
segments](https://user-images.githubusercontent.com/2322288/61318539-2d766400-a7ba-11e9-8db2-656d00ec4fe3.png)
   ![findFullyOvershadowed - 160 
segments](https://user-images.githubusercontent.com/2322288/61318543-2ea79100-a7ba-11e9-858a-58645de94d33.png)
   
   `findFullyOvershadowed` is very slow with segmentLock. This is because the 
implementation is not very optimized, especially the constructor of 
`PartitionHolder`. Here is the code snippet.
   
   ```java
     public PartitionHolder(List<PartitionChunk<T>> initialChunks)
     {
       this.overshadowableManager = new OvershadowableManager<>();
       for (PartitionChunk<T> chunk : initialChunks) {
         add(chunk);
       }
     }
   ```
   
   This could be improved by adding a bulk construction of 
OvershadowableManager, but `findFullyOvershadowed` is called only one place in 
production code, i.e., `ParallelSubIndexTask`. I think it could be done later 
rather than in this PR.
   
   #### IsOvershadowed
   
   ![isOvershadowed - varying # of 
segments](https://user-images.githubusercontent.com/2322288/61318549-31a28180-a7ba-11e9-98fc-f6b7da33fcaa.png)
   ![isOvershadowed - 160 
segments](https://user-images.githubusercontent.com/2322288/61318552-32d3ae80-a7ba-11e9-88da-1efeee4bd879.png)
   
   `isOvershadowed` shows a pretty similar throughput. The benchmark chooses a 
random one among timeChunks populated based on segmentGranularity. As a result, 
the benchmark always executes the below part in `isOvershadowed` method.
   
   ```java
         TimelineEntry entry = completePartitionsTimeline.get(interval);
         if (entry != null) {
           final int majorVersionCompare = versionComparator.compare(version, 
entry.getVersion());
           if (majorVersionCompare == 0) {
             for (PartitionChunk<ObjectType> chunk : entry.partitionHolder) {
               if (chunk.getObject().isOvershadow(object)) {
                 return true;
               }
             }
             return false;
           } else {
             return majorVersionCompare < 0;
           }
         }
   ```
   
   Since the number of visible segments to be iterated in the for loop is same 
with timeChunk and segment locks, their throughput should be similar.
   
   #### Add
   
   ![add - varying # of 
segments](https://user-images.githubusercontent.com/2322288/61318564-37986280-a7ba-11e9-9f32-655580ab3245.png)
   ![add - 160 
segments](https://user-images.githubusercontent.com/2322288/61318568-38c98f80-a7ba-11e9-85bb-e4eac6b3082b.png)
   
   For `add`, it's faster with segment lock because I think `TimelineEntry` 
should be created for each new segment of new major version. When using segment 
lock, new segment of new minor version can be added into the same 
`TimelineEntry`.
   
   #### Remove
   
   ![remove - varying # of 
segments](https://user-images.githubusercontent.com/2322288/61318573-3a935300-a7ba-11e9-81ca-57b0968f13c0.png)
   ![remove - 160 
segments](https://user-images.githubusercontent.com/2322288/61318578-3bc48000-a7ba-11e9-907b-cd073af734da.png)
   
   `remove` is slower with segment lock. This is expected since whenever a 
visible segment is removed, `OvershadowableManager` needs to check all 
overshadowed atomicUpdateGroups to determine the visible one.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to