w41ter commented on code in PR #46981:
URL: https://github.com/apache/doris/pull/46981#discussion_r1918287571
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/TableBinlog.java:
##########
@@ -76,6 +76,10 @@ public TBinlog getDummyBinlog() {
return binlogs.first();
}
+ public boolean isEmpty() {
+ return binlogs.size() == 1 && binlogs.first().getType() ==
TBinlogType.DUMMY;
+ }
Review Comment:
Acquire lock.
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/DBBinlog.java:
##########
@@ -287,17 +287,14 @@ public BinlogTombstone gc() {
if (dbBinlogConfig == null) {
LOG.error("db not found. dbId: {}", dbId);
return null;
+ } else if (!dbBinlogConfig.isEnable()) {
+ LOG.info("db binlog is disable, force gc all binlogs. dbId: {}",
dbId);
+ return dbBinlogDisableGc();
+ } else if (dbBinlogConfig.isEnable()) {
+ return dbBinlogEnableGc(dbBinlogConfig);
}
- BinlogTombstone tombstone;
- if (dbBinlogConfig.isEnable()) {
- // db binlog is enabled, only one binlogTombstones
- tombstone = dbBinlogEnableGc(dbBinlogConfig);
- } else {
- tombstone = dbBinlogDisableGc();
- }
-
- return tombstone;
+ return null;
Review Comment:
```suggestion
```
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/DBBinlog.java:
##########
@@ -324,6 +318,10 @@ private BinlogTombstone
collectTableTombstone(List<BinlogTombstone> tableTombsto
return dbTombstone;
}
+ public boolean isEmpty() {
+ return allBinlogs.size() == 1 && tableBinlogMap.isEmpty();
+ }
Review Comment:
Acquire lock
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/DBBinlog.java:
##########
@@ -340,7 +338,11 @@ private BinlogTombstone dbBinlogDisableGc() {
if (tombstone != null) {
tombstones.add(tombstone);
}
+ if (tableBinlog.isEmpty()) {
+ tableBinlogMap.remove(tableBinlog.getTableId());
+ }
Review Comment:
Acquire lock before checking `tableBinlog.isEmpty` and updating
`tableBinlogMap`.
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/BinlogManager.java:
##########
@@ -573,7 +573,11 @@ public List<BinlogTombstone> gc() {
if (dbTombstones != null) {
tombstones.add(dbTombstones);
}
+ if (dbBinlog.isEmpty()) {
+ dbBinlogMap.remove(dbBinlog.getDbId());
+ }
Review Comment:
Acquire lock before checking `dbBinlog` and updating `dbBinlogMap`
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/DBBinlog.java:
##########
@@ -287,17 +287,14 @@ public BinlogTombstone gc() {
if (dbBinlogConfig == null) {
LOG.error("db not found. dbId: {}", dbId);
return null;
+ } else if (!dbBinlogConfig.isEnable()) {
+ LOG.info("db binlog is disable, force gc all binlogs. dbId: {}",
dbId);
+ return dbBinlogDisableGc();
+ } else if (dbBinlogConfig.isEnable()) {
Review Comment:
```suggestion
} else {
```
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/DBBinlog.java:
##########
@@ -287,17 +287,14 @@ public BinlogTombstone gc() {
if (dbBinlogConfig == null) {
LOG.error("db not found. dbId: {}", dbId);
return null;
+ } else if (!dbBinlogConfig.isEnable()) {
+ LOG.info("db binlog is disable, force gc all binlogs. dbId: {}",
dbId);
Review Comment:
```suggestion
```
There is no forced GC for all binlogs here. Sometimes, only a table binlog
is enabled.
--
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
