pengxiangyu commented on code in PR #16488:
URL: https://github.com/apache/doris/pull/16488#discussion_r1099860083
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java:
##########
@@ -335,48 +337,81 @@ private boolean needSync(Replica replicaInFe, TTabletInfo
backendTabletInfo) {
return false;
}
- private boolean needChangeCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo) {
- if (!beTabletInfo.isIsCooldown()) {
- return false;
- }
- // check cooldown type in fe and be, they need to be the same.
- if (tabletMeta.getCooldownReplicaId() !=
beTabletInfo.getCooldownReplicaId()) {
- LOG.warn("cooldownReplicaId is wrong for tablet: {}, Fe: {}, Be:
{}", beTabletInfo.getTabletId(),
- tabletMeta.getCooldownReplicaId(),
beTabletInfo.getCooldownReplicaId());
- return true;
+ private void handleCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo,
+ List<CooldownConf> cooldownConfToPush, List<CooldownConf>
cooldownConfToUpdate) {
+ if (!beTabletInfo.isSetCooldownReplicaId()) {
+ return;
}
- // check cooldown type in one tablet, One UPLOAD_DATA is needed in the
replicas.
- long stamp = readLock();
+ Tablet tablet;
try {
- boolean replicaInvalid = true;
- Map<Long, Replica> replicaMap =
replicaMetaTable.row(beTabletInfo.getTabletId());
- for (Map.Entry<Long, Replica> entry : replicaMap.entrySet()) {
- if (entry.getValue().getId() ==
beTabletInfo.getCooldownReplicaId()) {
- replicaInvalid = false;
- break;
- }
+ OlapTable table = (OlapTable)
Env.getCurrentInternalCatalog().getDbNullable(tabletMeta.getDbId())
+ .getTable(tabletMeta.getTableId())
+ .get();
+ table.readLock();
+ try {
+ tablet =
table.getPartition(tabletMeta.getPartitionId()).getIndex(tabletMeta.getIndexId())
+ .getTablet(beTabletInfo.tablet_id);
+ } finally {
+ table.readUnlock();
}
- if (replicaInvalid) {
- return true;
+ } catch (RuntimeException e) {
+ LOG.warn("failed to get tablet. tabletId={}",
beTabletInfo.tablet_id);
+ return;
+ }
+ Pair<Long, Long> cooldownConf = tablet.getCooldownConf();
Review Comment:
Create a class named CooldownConf is better, it will be more readability.
And it is easier to add new element to CooldownConf.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java:
##########
@@ -335,48 +337,81 @@ private boolean needSync(Replica replicaInFe, TTabletInfo
backendTabletInfo) {
return false;
}
- private boolean needChangeCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo) {
- if (!beTabletInfo.isIsCooldown()) {
- return false;
- }
- // check cooldown type in fe and be, they need to be the same.
- if (tabletMeta.getCooldownReplicaId() !=
beTabletInfo.getCooldownReplicaId()) {
- LOG.warn("cooldownReplicaId is wrong for tablet: {}, Fe: {}, Be:
{}", beTabletInfo.getTabletId(),
- tabletMeta.getCooldownReplicaId(),
beTabletInfo.getCooldownReplicaId());
- return true;
+ private void handleCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo,
+ List<CooldownConf> cooldownConfToPush, List<CooldownConf>
cooldownConfToUpdate) {
+ if (!beTabletInfo.isSetCooldownReplicaId()) {
+ return;
}
- // check cooldown type in one tablet, One UPLOAD_DATA is needed in the
replicas.
- long stamp = readLock();
+ Tablet tablet;
try {
- boolean replicaInvalid = true;
- Map<Long, Replica> replicaMap =
replicaMetaTable.row(beTabletInfo.getTabletId());
- for (Map.Entry<Long, Replica> entry : replicaMap.entrySet()) {
- if (entry.getValue().getId() ==
beTabletInfo.getCooldownReplicaId()) {
- replicaInvalid = false;
- break;
- }
+ OlapTable table = (OlapTable)
Env.getCurrentInternalCatalog().getDbNullable(tabletMeta.getDbId())
+ .getTable(tabletMeta.getTableId())
+ .get();
+ table.readLock();
+ try {
+ tablet =
table.getPartition(tabletMeta.getPartitionId()).getIndex(tabletMeta.getIndexId())
+ .getTablet(beTabletInfo.tablet_id);
+ } finally {
+ table.readUnlock();
}
- if (replicaInvalid) {
- return true;
+ } catch (RuntimeException e) {
+ LOG.warn("failed to get tablet. tabletId={}",
beTabletInfo.tablet_id);
+ return;
+ }
+ Pair<Long, Long> cooldownConf = tablet.getCooldownConf();
+ if (beTabletInfo.getCooldownTerm() > cooldownConf.second) { // should
not be here
+ LOG.warn("report cooldownTerm({}) > cooldownTerm in
TabletMeta({}), tabletId={}",
+ beTabletInfo.getCooldownTerm(), cooldownConf.second,
beTabletInfo.tablet_id);
+ return;
+ }
+
+ if (cooldownConf.first <= 0) { // invalid cooldownReplicaId
+ CooldownConf conf = new CooldownConf(tabletMeta.getDbId(),
tabletMeta.getTableId(),
+ tabletMeta.getPartitionId(), tabletMeta.getIndexId(),
beTabletInfo.tablet_id, cooldownConf.second);
+ synchronized (cooldownConfToUpdate) {
Review Comment:
handleCooldownConf is single thread. synchronized is not needed.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java:
##########
@@ -335,48 +337,81 @@ private boolean needSync(Replica replicaInFe, TTabletInfo
backendTabletInfo) {
return false;
}
- private boolean needChangeCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo) {
- if (!beTabletInfo.isIsCooldown()) {
- return false;
- }
- // check cooldown type in fe and be, they need to be the same.
- if (tabletMeta.getCooldownReplicaId() !=
beTabletInfo.getCooldownReplicaId()) {
- LOG.warn("cooldownReplicaId is wrong for tablet: {}, Fe: {}, Be:
{}", beTabletInfo.getTabletId(),
- tabletMeta.getCooldownReplicaId(),
beTabletInfo.getCooldownReplicaId());
- return true;
+ private void handleCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo,
+ List<CooldownConf> cooldownConfToPush, List<CooldownConf>
cooldownConfToUpdate) {
+ if (!beTabletInfo.isSetCooldownReplicaId()) {
+ return;
}
- // check cooldown type in one tablet, One UPLOAD_DATA is needed in the
replicas.
- long stamp = readLock();
+ Tablet tablet;
try {
- boolean replicaInvalid = true;
- Map<Long, Replica> replicaMap =
replicaMetaTable.row(beTabletInfo.getTabletId());
- for (Map.Entry<Long, Replica> entry : replicaMap.entrySet()) {
- if (entry.getValue().getId() ==
beTabletInfo.getCooldownReplicaId()) {
- replicaInvalid = false;
- break;
- }
+ OlapTable table = (OlapTable)
Env.getCurrentInternalCatalog().getDbNullable(tabletMeta.getDbId())
+ .getTable(tabletMeta.getTableId())
+ .get();
+ table.readLock();
+ try {
+ tablet =
table.getPartition(tabletMeta.getPartitionId()).getIndex(tabletMeta.getIndexId())
+ .getTablet(beTabletInfo.tablet_id);
+ } finally {
+ table.readUnlock();
}
- if (replicaInvalid) {
- return true;
+ } catch (RuntimeException e) {
+ LOG.warn("failed to get tablet. tabletId={}",
beTabletInfo.tablet_id);
+ return;
+ }
+ Pair<Long, Long> cooldownConf = tablet.getCooldownConf();
+ if (beTabletInfo.getCooldownTerm() > cooldownConf.second) { // should
not be here
+ LOG.warn("report cooldownTerm({}) > cooldownTerm in
TabletMeta({}), tabletId={}",
+ beTabletInfo.getCooldownTerm(), cooldownConf.second,
beTabletInfo.tablet_id);
+ return;
+ }
+
+ if (cooldownConf.first <= 0) { // invalid cooldownReplicaId
+ CooldownConf conf = new CooldownConf(tabletMeta.getDbId(),
tabletMeta.getTableId(),
+ tabletMeta.getPartitionId(), tabletMeta.getIndexId(),
beTabletInfo.tablet_id, cooldownConf.second);
+ synchronized (cooldownConfToUpdate) {
+ cooldownConfToUpdate.add(conf);
}
- } finally {
- readUnlock(stamp);
+ return;
+ }
+
+ // validate replica is active
+ Map<Long, Replica> replicaMap =
replicaMetaTable.row(beTabletInfo.getTabletId());
+ if (replicaMap.isEmpty()) {
+ return;
+ }
+ boolean replicaInvalid = true;
+ for (Replica replica : replicaMap.values()) {
+ if (replica.getId() == cooldownConf.first) {
+ replicaInvalid = false;
+ break;
+ }
+ }
+ if (replicaInvalid) {
+ CooldownConf conf = new CooldownConf(tabletMeta.getDbId(),
tabletMeta.getTableId(),
+ tabletMeta.getPartitionId(), tabletMeta.getIndexId(),
beTabletInfo.tablet_id, cooldownConf.second);
+ synchronized (cooldownConfToUpdate) {
+ cooldownConfToUpdate.add(conf);
+ }
+ return;
+ }
+
+ if (cooldownConf.first != beTabletInfo.getCooldownReplicaId()) {
+ CooldownConf conf = new CooldownConf(beTabletInfo.tablet_id,
cooldownConf.first, cooldownConf.second);
+ synchronized (cooldownConfToPush) {
Review Comment:
handleCooldownConf is single thread. synchronized is not needed.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java:
##########
@@ -335,48 +337,81 @@ private boolean needSync(Replica replicaInFe, TTabletInfo
backendTabletInfo) {
return false;
}
- private boolean needChangeCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo) {
- if (!beTabletInfo.isIsCooldown()) {
- return false;
- }
- // check cooldown type in fe and be, they need to be the same.
- if (tabletMeta.getCooldownReplicaId() !=
beTabletInfo.getCooldownReplicaId()) {
- LOG.warn("cooldownReplicaId is wrong for tablet: {}, Fe: {}, Be:
{}", beTabletInfo.getTabletId(),
- tabletMeta.getCooldownReplicaId(),
beTabletInfo.getCooldownReplicaId());
- return true;
+ private void handleCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo,
+ List<CooldownConf> cooldownConfToPush, List<CooldownConf>
cooldownConfToUpdate) {
+ if (!beTabletInfo.isSetCooldownReplicaId()) {
+ return;
}
- // check cooldown type in one tablet, One UPLOAD_DATA is needed in the
replicas.
- long stamp = readLock();
+ Tablet tablet;
try {
- boolean replicaInvalid = true;
- Map<Long, Replica> replicaMap =
replicaMetaTable.row(beTabletInfo.getTabletId());
- for (Map.Entry<Long, Replica> entry : replicaMap.entrySet()) {
- if (entry.getValue().getId() ==
beTabletInfo.getCooldownReplicaId()) {
- replicaInvalid = false;
- break;
- }
+ OlapTable table = (OlapTable)
Env.getCurrentInternalCatalog().getDbNullable(tabletMeta.getDbId())
+ .getTable(tabletMeta.getTableId())
+ .get();
+ table.readLock();
+ try {
+ tablet =
table.getPartition(tabletMeta.getPartitionId()).getIndex(tabletMeta.getIndexId())
+ .getTablet(beTabletInfo.tablet_id);
+ } finally {
+ table.readUnlock();
}
- if (replicaInvalid) {
- return true;
+ } catch (RuntimeException e) {
+ LOG.warn("failed to get tablet. tabletId={}",
beTabletInfo.tablet_id);
+ return;
+ }
+ Pair<Long, Long> cooldownConf = tablet.getCooldownConf();
+ if (beTabletInfo.getCooldownTerm() > cooldownConf.second) { // should
not be here
+ LOG.warn("report cooldownTerm({}) > cooldownTerm in
TabletMeta({}), tabletId={}",
+ beTabletInfo.getCooldownTerm(), cooldownConf.second,
beTabletInfo.tablet_id);
+ return;
+ }
+
+ if (cooldownConf.first <= 0) { // invalid cooldownReplicaId
+ CooldownConf conf = new CooldownConf(tabletMeta.getDbId(),
tabletMeta.getTableId(),
+ tabletMeta.getPartitionId(), tabletMeta.getIndexId(),
beTabletInfo.tablet_id, cooldownConf.second);
+ synchronized (cooldownConfToUpdate) {
+ cooldownConfToUpdate.add(conf);
}
- } finally {
- readUnlock(stamp);
+ return;
+ }
+
+ // validate replica is active
+ Map<Long, Replica> replicaMap =
replicaMetaTable.row(beTabletInfo.getTabletId());
+ if (replicaMap.isEmpty()) {
+ return;
+ }
+ boolean replicaInvalid = true;
+ for (Replica replica : replicaMap.values()) {
+ if (replica.getId() == cooldownConf.first) {
+ replicaInvalid = false;
+ break;
+ }
+ }
+ if (replicaInvalid) {
+ CooldownConf conf = new CooldownConf(tabletMeta.getDbId(),
tabletMeta.getTableId(),
+ tabletMeta.getPartitionId(), tabletMeta.getIndexId(),
beTabletInfo.tablet_id, cooldownConf.second);
+ synchronized (cooldownConfToUpdate) {
Review Comment:
handleCooldownConf is single thread. synchronized is not needed.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
