github-actions[bot] commented on code in PR #63864:
URL: https://github.com/apache/doris/pull/63864#discussion_r3556187275
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowTrashCommand.java:
##########
@@ -42,6 +43,17 @@
* show trash command
*/
public class ShowTrashCommand extends ShowCommand {
+ private static final ShowResultSetMetaData META_DATA =
ShowResultSetMetaData.builder()
+ .addColumn(new Column("BackendId",
ScalarType.createType(PrimitiveType.BIGINT)))
+ .addColumn(new Column("Backend", ScalarType.createStringType()))
+ .addColumn(new Column("RootPath", ScalarType.createStringType()))
+ .addColumn(new Column("State", ScalarType.createStringType()))
+ .addColumn(new Column("TrashUsedCapacity",
ScalarType.createStringType()))
Review Comment:
This column cannot safely be `BIGINT` while the new fallback path can emit
`"N/A"` for the same slot. When `getDiskTrashUsedCapacity()` is unavailable,
`TrashProcDir.addFallbackRow()` fills `TrashFileNum` with `"N/A"`; if the
client executes `SHOW TRASH` through the prepared-statement path,
`StmtExecutor.sendBinaryResultRow()` sees the BIGINT metadata and calls
`Long.parseLong(item)`, so the diagnostic fallback row fails to serialize
instead of being returned. Please keep the metadata and all row values
consistent, e.g. make this column a string like the other placeholder-capable
diagnostic columns or emit a numeric/null value on fallback.
##########
fe/fe-core/src/main/java/org/apache/doris/common/proc/TrashProcDir.java:
##########
@@ -99,20 +102,61 @@ public static void getTrashInfo(List<Backend> backends,
List<List<String>> infos
}
}
- List<String> backendInfo = new ArrayList<>();
- backendInfo.add(String.valueOf(backend.getId()));
-
backendInfo.add(NetUtils.getHostPortInAccessibleFormat(backend.getHost(),
backend.getHeartbeatPort()));
- if (trashUsedCapacityB != null) {
+ if (CollectionUtils.isEmpty(diskTrashInfos)) {
+ addFallbackRow(backend, infos);
+ continue;
+ }
+ for (TDiskTrashInfo diskTrashInfo : diskTrashInfos) {
+ List<String> rowInfo = new ArrayList<>();
+ rowInfo.add(String.valueOf(backend.getId()));
+
rowInfo.add(NetUtils.getHostPortInAccessibleFormat(backend.getHost(),
backend.getHeartbeatPort()));
+ rowInfo.add(diskTrashInfo.getRootPath());
+ rowInfo.add(diskTrashInfo.getState());
+
+ long trashUsedCapacityB = diskTrashInfo.getTrashUsedCapacity();
Pair<Double, String> trashUsedCapacity =
DebugUtil.getByteUint(trashUsedCapacityB);
-
backendInfo.add(DebugUtil.DECIMAL_FORMAT_SCALE_3.format(trashUsedCapacity.first)
+ " "
+
rowInfo.add(DebugUtil.DECIMAL_FORMAT_SCALE_3.format(trashUsedCapacity.first) +
" "
+ trashUsedCapacity.second);
- } else {
Review Comment:
Treating an unset `trash_file_num` as `"0"` makes mixed-version results
misleading. Because field 4 is newly added and optional, a new FE can talk to
an old BE that returns valid per-disk trash rows without this field set; this
code will show every such disk as having exactly zero trash files even if the
BE simply did not report the count. The other new optional fields are rendered
as unknown/empty when unset, and `TrashProcNode` has the same `"0"` fallback,
so please handle `trash_file_num` as unknown too and update the compatibility
test that currently locks in `"0"`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
