You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/06/10 05:56:24 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request #3815: Optimize the logic for getting tabletMeta from TabletInvertIndex to reduce frequence of getting read lock
caiconghui opened a new pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815
This PR is to optimize the logic for getting tabletMeta from TabletInvertIndex to reduce frequence of getting read lock
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequency of getting read lock
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815#discussion_r438034697
##########
File path: fe/src/main/java/org/apache/doris/master/ReportHandler.java
##########
@@ -388,21 +389,26 @@ private static void sync(Map<Long, TTablet> backendTablets, ListMultimap<Long, L
List<Long> tabletIds = tabletSyncMap.get(dbId);
LOG.info("before sync tablets in db[{}]. report num: {}. backend[{}]",
dbId, tabletIds.size(), backendId);
-
- for (Long tabletId : tabletIds) {
- long tableId = invertedIndex.getTableId(tabletId);
+ List<TabletMeta> tabletMetaList = invertedIndex.getTabletMetaList(tabletIds);
Review comment:
Have you tested its performance? There may be millions of tablet in `tabletIds`. I am not sure how long it will take to process that much of tablets.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequency of getting read lock
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815#discussion_r438057994
##########
File path: fe/src/main/java/org/apache/doris/master/ReportHandler.java
##########
@@ -388,21 +389,26 @@ private static void sync(Map<Long, TTablet> backendTablets, ListMultimap<Long, L
List<Long> tabletIds = tabletSyncMap.get(dbId);
LOG.info("before sync tablets in db[{}]. report num: {}. backend[{}]",
dbId, tabletIds.size(), backendId);
-
- for (Long tabletId : tabletIds) {
- long tableId = invertedIndex.getTableId(tabletId);
+ List<TabletMeta> tabletMetaList = invertedIndex.getTabletMetaList(tabletIds);
Review comment:
cost for getting a list of size for 1000000 from map is about 40 ~ 50ms, here we only process tablets in one be and which need to sync, so i think 1000000 for testing is large enough
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequency of getting read lock
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815#discussion_r438767143
##########
File path: fe/src/main/java/org/apache/doris/master/ReportHandler.java
##########
@@ -388,21 +389,26 @@ private static void sync(Map<Long, TTablet> backendTablets, ListMultimap<Long, L
List<Long> tabletIds = tabletSyncMap.get(dbId);
LOG.info("before sync tablets in db[{}]. report num: {}. backend[{}]",
dbId, tabletIds.size(), backendId);
-
- for (Long tabletId : tabletIds) {
- long tableId = invertedIndex.getTableId(tabletId);
+ List<TabletMeta> tabletMetaList = invertedIndex.getTabletMetaList(tabletIds);
Review comment:
OK
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] imay merged pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequency of getting read lock
Posted by GitBox <gi...@apache.org>.
imay merged pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequence of getting read lock
Posted by GitBox <gi...@apache.org>.
caiconghui commented on pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815#issuecomment-641743439
for #3816
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequency of getting read lock
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815#discussion_r438769259
##########
File path: fe/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java
##########
@@ -298,57 +262,28 @@ public Long getTabletIdByReplica(long replicaId) {
}
}
- public long getPartitionId(long tabletId) {
- readLock();
- try {
- if (tabletMetaMap.containsKey(tabletId)) {
- return tabletMetaMap.get(tabletId).getPartitionId();
- }
- return NOT_EXIST_VALUE;
- } finally {
- readUnlock();
- }
- }
-
- public long getIndexId(long tabletId) {
+ public TabletMeta getTabletMeta(long tabletId) {
readLock();
try {
- if (tabletMetaMap.containsKey(tabletId)) {
- return tabletMetaMap.get(tabletId).getIndexId();
- }
- return NOT_EXIST_VALUE;
+ return tabletMetaMap.get(tabletId);
} finally {
readUnlock();
}
}
- public int getEffectiveSchemaHash(long tabletId) {
- // always get old schema hash(as effective one)
+ public List<TabletMeta> getTabletMetaList(List<Long> tabletIdList) {
Review comment:
It is very error-prone to result a list will null in it.
How about using `getOrDefault()` method, and return an `NonExistTabletMeta` is not found?
And `NonExistTabletMeta` can be a `final static` member with all id as `NON_EXIST_VALUE` in it.
In this way, we don't have to check null each time we visit the returned list.
##########
File path: fe/src/main/java/org/apache/doris/master/ReportHandler.java
##########
@@ -388,21 +389,26 @@ private static void sync(Map<Long, TTablet> backendTablets, ListMultimap<Long, L
List<Long> tabletIds = tabletSyncMap.get(dbId);
LOG.info("before sync tablets in db[{}]. report num: {}. backend[{}]",
dbId, tabletIds.size(), backendId);
-
- for (Long tabletId : tabletIds) {
- long tableId = invertedIndex.getTableId(tabletId);
+ List<TabletMeta> tabletMetaList = invertedIndex.getTabletMetaList(tabletIds);
+ for (int i = 0; i < tabletMetaList.size(); i++) {
+ TabletMeta tabletMeta = tabletMetaList.get(i);
+ if (tabletMeta == null) {
+ continue;
+ }
+ long tabletId = tabletIds.get(i);
+ long tableId = tabletMeta.getTableId();
OlapTable olapTable = (OlapTable) db.getTable(tableId);
if (olapTable == null) {
continue;
}
- long partitionId = invertedIndex.getPartitionId(tabletId);
+ long partitionId = tabletMeta.getPartitionId();
Partition partition = olapTable.getPartition(partitionId);
if (partition == null) {
continue;
}
- long indexId = invertedIndex.getIndexId(tabletId);
+ long indexId = tabletMeta.getPartitionId();
Review comment:
```suggestion
long indexId = tabletMeta.getIndexId();
```
Please double check again for similar errors.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequency of getting read lock
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815#discussion_r438852344
##########
File path: fe/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java
##########
@@ -298,57 +262,28 @@ public Long getTabletIdByReplica(long replicaId) {
}
}
- public long getPartitionId(long tabletId) {
- readLock();
- try {
- if (tabletMetaMap.containsKey(tabletId)) {
- return tabletMetaMap.get(tabletId).getPartitionId();
- }
- return NOT_EXIST_VALUE;
- } finally {
- readUnlock();
- }
- }
-
- public long getIndexId(long tabletId) {
+ public TabletMeta getTabletMeta(long tabletId) {
readLock();
try {
- if (tabletMetaMap.containsKey(tabletId)) {
- return tabletMetaMap.get(tabletId).getIndexId();
- }
- return NOT_EXIST_VALUE;
+ return tabletMetaMap.get(tabletId);
} finally {
readUnlock();
}
}
- public int getEffectiveSchemaHash(long tabletId) {
- // always get old schema hash(as effective one)
+ public List<TabletMeta> getTabletMetaList(List<Long> tabletIdList) {
Review comment:
ok
##########
File path: fe/src/main/java/org/apache/doris/master/ReportHandler.java
##########
@@ -388,21 +389,26 @@ private static void sync(Map<Long, TTablet> backendTablets, ListMultimap<Long, L
List<Long> tabletIds = tabletSyncMap.get(dbId);
LOG.info("before sync tablets in db[{}]. report num: {}. backend[{}]",
dbId, tabletIds.size(), backendId);
-
- for (Long tabletId : tabletIds) {
- long tableId = invertedIndex.getTableId(tabletId);
+ List<TabletMeta> tabletMetaList = invertedIndex.getTabletMetaList(tabletIds);
+ for (int i = 0; i < tabletMetaList.size(); i++) {
+ TabletMeta tabletMeta = tabletMetaList.get(i);
+ if (tabletMeta == null) {
+ continue;
+ }
+ long tabletId = tabletIds.get(i);
+ long tableId = tabletMeta.getTableId();
OlapTable olapTable = (OlapTable) db.getTable(tableId);
if (olapTable == null) {
continue;
}
- long partitionId = invertedIndex.getPartitionId(tabletId);
+ long partitionId = tabletMeta.getPartitionId();
Partition partition = olapTable.getPartition(partitionId);
if (partition == null) {
continue;
}
- long indexId = invertedIndex.getIndexId(tabletId);
+ long indexId = tabletMeta.getPartitionId();
Review comment:
fix
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3815: Optimize the logic for getting TabletMeta from TabletInvertedIndex to reduce frequency of getting read lock
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3815:
URL: https://github.com/apache/incubator-doris/pull/3815#discussion_r438057994
##########
File path: fe/src/main/java/org/apache/doris/master/ReportHandler.java
##########
@@ -388,21 +389,26 @@ private static void sync(Map<Long, TTablet> backendTablets, ListMultimap<Long, L
List<Long> tabletIds = tabletSyncMap.get(dbId);
LOG.info("before sync tablets in db[{}]. report num: {}. backend[{}]",
dbId, tabletIds.size(), backendId);
-
- for (Long tabletId : tabletIds) {
- long tableId = invertedIndex.getTableId(tabletId);
+ List<TabletMeta> tabletMetaList = invertedIndex.getTabletMetaList(tabletIds);
Review comment:
cost for getting a list of size for 1000000 from map is about 40 ~ 50ms, here we only preocess tablets in one be and which need to sync, so i think 1000000 for testing is large enough
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org