adoroszlai commented on code in PR #5407:
URL: https://github.com/apache/ozone/pull/5407#discussion_r1360760845
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -202,11 +209,11 @@ private OzoneManagerDoubleBuffer(OMMetadataManager
omMetadataManager,
OzoneManagerDoubleBufferMetrics.create();
this.indexToTerm = indexToTerm;
this.flushNotifier = flushNotifier;
-
+ this.threadPrefix = (threadPrefix == null) ? "" : threadPrefix;
Review Comment:
We can avoid `null` prefix by setting `""` as initial value in the builder:
https://github.com/apache/ozone/blob/02d3f30030f0b95e430de366d6be8af9022d68bf/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java#L128
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2113,7 +2116,7 @@ private void startTrashEmptier(Configuration conf) throws
IOException {
(PrivilegedExceptionAction<FileSystem>)
() -> new TrashOzoneFileSystem(i));
this.emptier = new Thread(new OzoneTrash(fs, conf, this).
- getEmptier(), "Trash Emptier");
+ getEmptier(), threadPrefix + "Trash Emptier");
Review Comment:
Nit: it would be nice to get rid of the space in thread names. Makes
cutting specific columns from log by whitespace a lot easier.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java:
##########
@@ -286,6 +287,10 @@ protected class Emptier implements Runnable {
trashEmptierCorePoolSize, 1,
TimeUnit.SECONDS, new ArrayBlockingQueue<>(1024),
new ThreadPoolExecutor.CallerRunsPolicy());
+
+ // Set a custom thread name with the provided prefix
+ Thread.currentThread()
+ .setName(threadNamePrefix + "-" + Thread.currentThread().getName());
Review Comment:
I don't think we should update the current thread name here. `Emptier` is a
thread, but it is being created in another thread. We should not change that
thread's name here.
To help explain with some extra log:
```
[main] INFO om.TrashPolicyOzone (TrashPolicyOzone.java:<init>(292)) - ZZZ
updating thread name main -> om1--main
[om1--main] INFO om.TrashPolicyOzone (TrashPolicyOzone.java:<init>(292)) -
ZZZ updating thread name om1--main -> om1--om1--main
[om1--om1--main] INFO om.TrashPolicyOzone
(TrashPolicyOzone.java:<init>(292)) - ZZZ updating thread name om1--om1--main
-> om1--om1--om1--main
[om1--om1--om1--main] INFO om.TrashPolicyOzone
(TrashPolicyOzone.java:<init>(292)) - ZZZ updating thread name
om1--om1--om1--main -> om1--om1--om1--om1--main
```
However, we should provide a `ThreadFactory` with thread name format (e.g.
`threadNamePrefix + "TrashEmptier"`) to the `executor` a few lines above.
https://github.com/apache/ozone/blob/02d3f30030f0b95e430de366d6be8af9022d68bf/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java#L286-L289
Also, prefix already has `-` (unless it's empty).
--
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]
