[
https://issues.apache.org/jira/browse/PHOENIX-5494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16974811#comment-16974811
]
chenglei edited comment on PHOENIX-5494 at 11/15/19 5:51 AM:
-------------------------------------------------------------
[~larsh], sorry for later response. I have already made a patch yesteray using
{{SkipScanFilter}} in my production environment(which is hbase1.4.10)to solve
this problem.
After reading [~kozdemir] 's patch, there are some suggestions from my patch
viewpoint which I think we can do better:
1. Scan.startRow and Scan.endRow should be set to improve the scanner
performance in additional to just using the MultiRowRangeFilter(In fact we can
also use SkipScanFilter here):
{code:java}
s.setFilter(new MultiRowRangeFilter(ranges));
s.setTimeRange(0, ts);
Region region = this.env.getRegion();
try (RegionScanner scanner = region.getScanner(s)) {
{code}
2. In additional to normal updates, we can also using the this optimization
when replaying write.
3. {{LocalTable}} is global region-scope, so if we add a member variable
{{LocalTable.results}} to make it stateful, there may be many concurrent
issues. I think we would better create a new class(for example
CachedLocalTable) to hold the results we pre-scannered and make the object as a
local variable,passing it to downstream {{IndexBuilder.getIndexUpdate}} to
avoid concurrent issues completely, and {{IndexBuildManager.removeRowStates}}
is no need.
such as:
IndexBuildManager.getIndexUpdates
{code:java}
for (Mutation m : mutations) {
Collection<Pair<Mutation, byte[]>> updates = delegate.getIndexUpdate(m,
indexMetaData, cachedLocalTable);
if (PhoenixIndexMetaData.isIndexRebuild(m.getAttributesMap())) {
for (Pair<Mutation, byte[]> update : updates) {
update.getFirst().setAttribute(BaseScannerRegionObserver.REPLAY_WRITES,
BaseScannerRegionObserver.REPLAY_INDEX_REBUILD_WRITES);
}
}
{code}
NonTxIndexBuilder.getIndexUpdate
{code:java}
@Override
public Collection<Pair<Mutation, byte[]>> getIndexUpdate(Mutation mutation,
IndexMetaData indexMetaData, LocalHBaseState localHBaseState) throws
IOException {
// create a state manager, so we can manage each batch
LocalTableState state = new LocalTableState(localHBaseState, mutation);
// build the index updates for each group
IndexUpdateManager manager = new IndexUpdateManager(indexMetaData);
batchMutationAndAddUpdates(manager, state, mutation, indexMetaData);
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Found index updates for Mutation: " + mutation + "\n"
+ manager);
}
return manager.toMap();
}
{code}
4. I think made the modifications sealed to {{IndexBuildManager}} is better,
{{IndexRegionObserver}} is no need to know the pre-scan, so this optimization
can also be used to another class such as the old {{Indexer}} coprocessor.
was (Author: comnetwork):
[~larsh], sorry for later response. I have already made a patch yesteray using
{{SkipScanFilter}} in my production environment(which is hbase1.4.10)to solve
this problem.
After reading [~kozdemir] 's patch, there are some suggestions from my patch
viewpoint which I think we can do better:
1. Scan.startRow and Scan.endRow should be set to improve the scanner
performance in additional to just using the MultiRowRangeFilter:
{code:java}
s.setFilter(new MultiRowRangeFilter(ranges));
s.setTimeRange(0, ts);
Region region = this.env.getRegion();
try (RegionScanner scanner = region.getScanner(s)) {
{code}
2. In additional to normal updates, we can also using the this optimization
when replaying write.
3. {{LocalTable}} is global region-scope, so if we add a member variable
{{LocalTable.results}} to make it stateful, there may be many concurrent
issues. I think we would better create a new class(for example
CachedLocalTable) to hold the results we pre-scannered and make the object as a
local variable,passing it to downstream {{IndexBuilder.getIndexUpdate}} to
avoid concurrent issues completely, and {{IndexBuildManager.removeRowStates}}
is no need.
such as:
IndexBuildManager.getIndexUpdates
{code:java}
for (Mutation m : mutations) {
Collection<Pair<Mutation, byte[]>> updates = delegate.getIndexUpdate(m,
indexMetaData, cachedLocalTable);
if (PhoenixIndexMetaData.isIndexRebuild(m.getAttributesMap())) {
for (Pair<Mutation, byte[]> update : updates) {
update.getFirst().setAttribute(BaseScannerRegionObserver.REPLAY_WRITES,
BaseScannerRegionObserver.REPLAY_INDEX_REBUILD_WRITES);
}
}
{code}
NonTxIndexBuilder.getIndexUpdate
{code:java}
@Override
public Collection<Pair<Mutation, byte[]>> getIndexUpdate(Mutation mutation,
IndexMetaData indexMetaData, LocalHBaseState localHBaseState) throws
IOException {
// create a state manager, so we can manage each batch
LocalTableState state = new LocalTableState(localHBaseState, mutation);
// build the index updates for each group
IndexUpdateManager manager = new IndexUpdateManager(indexMetaData);
batchMutationAndAddUpdates(manager, state, mutation, indexMetaData);
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Found index updates for Mutation: " + mutation + "\n"
+ manager);
}
return manager.toMap();
}
{code}
4. I think made the modifications sealed to {{IndexBuildManager}} is better,
{{IndexRegionObserver}} is no need to know the pre-scan, so this optimization
can also be used to another class such as the old {{Indexer}} coprocessor.
> Batched, mutable Index updates are unnecessarily run one-by-one
> ---------------------------------------------------------------
>
> Key: PHOENIX-5494
> URL: https://issues.apache.org/jira/browse/PHOENIX-5494
> Project: Phoenix
> Issue Type: Improvement
> Reporter: Lars Hofhansl
> Assignee: Kadir OZDEMIR
> Priority: Major
> Labels: performance
> Attachments: 5494-4.x-HBase-1.5.txt, PHOENIX-5494.master.001.patch,
> PHOENIX-5494.master.002.patch, PHOENIX-5494.master.003.patch,
> Screenshot_20191110_160243.png, Screenshot_20191110_160351.png,
> Screenshot_20191110_161453.png
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> I just noticed that index updates on mutable tables retrieve their deletes
> (to invalidate the old index entry) one-by-one.
> For batches, this can be *the* major time spent during an index update. The
> cost is mostly incured by the repeated setup (and seeking) of the new region
> scanner (for each row).
> We can instead do a skip scan and get all updates in a single scan per region.
> (Logically that is simple, but it will require some refactoring)
> I won't be getting to this, but recording it here in case someone feels
> inclined.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
