stoty commented on PR #7892:
URL: https://github.com/apache/hadoop/pull/7892#issuecomment-3270609216
I've spent some more tim on this @szetszwo , and I am even more convinced
that using the Builder approach is NOT a good universal solution.
I've implemented something close to your example above, then I randomy chose
org.apache.hadoop.hdfs.server.namenode.ha.EditLogTailer to test it.
Two problems became immediately apparent with the builder approach:
1. EditLogTailerThread has an internal state and a setShouldRun() method to
set it. This immediately makes the Builder approach you've shown a non-starter,
as the thread created by the Builder is of a different Type, and does not have
a setShouldRun() method (nor the required internal state)
2. An EditLogTailerThread member is defined in the class. Even if we could
get away with changing the type (which in many cases we can do), we'd still
have to either
a.) change the type to Thread, thereby weakening the type checks
b.) add another member for the wrapped thread, in which case we need go over
the code, and carefully check which member we need to use where, adding a lot
of possibilities for mistakes.
That means that only the Threads which set a Runner can be converted easily,
the ones which subclass Thread would need to be evaulated on a case-by-case
basis, which is exactly what I don't want to do due to both the time
requirement and the error-prone nature of the process.
@pan3793 @slfan1989 @jojochuang @cnauroth @steveloughran @Hexiaoqiao
@ayushtkn @szetszwo
Please review the current state of #7919 which is the same patch as this
split into three and with some minor improvements.
My plan is to rebase/reorder the #7919 . Please check #7919
* Do you think we should implement and use Thread Builder approach in this
patch ? (see my reservations above)
* Is the way I split the patch acceptable (patches without JIRA id will be
merged into the other ones)
* What other changes (if any) do you think are necessary for patch to be
accepted M
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
