[GitHub] [incubator-druid] yongmeibian commented on issue #8956: concurrent access to charsetfix method throws IllegalStateException bugfix#8967

[GitBox <gi...@apache.org>]

yongmeibian commented on issue #8956: concurrent access to charsetfix method throws IllegalStateException bugfix#8967
URL: https://github.com/apache/incubator-druid/pull/8956#issuecomment-562556737
 
 
   Thx for ur advice.
   IMO,
   1.
   > "For performance reasons, I need to process multiple rows concurrently in SeekableStreamIndexTaskRunner.runInternal()." described  the background I encountered this issue.
   Actually，I mean the method  charsetFix() is not thread safe which can be simply fixed by using "CharsetsEncoder enc" as a local variable rather than a member variable.
   2.
   As "CharsetEncoder enc" is only used in charsetFix() and charsetFix () can not be accessed concurrently just due to the member variable CharsetEncoder enc. As an  open source project, using "CharsetEncoder enc“ as a local variable may be more extensible  and friendly.
   3.
   Teaching fish to swim, using "CharsetEncoder enc" as a local variable does cause createing CharsetEncoder many times. But,I think it is a better choice under the trade-off between extensibility and memory space .To some extent,The statement “private final CharsetEncoder enc = StandardCharsets.UTF_8.newEncoder(); “ is executed before the  constructor (although someone do not want to call charsetFix() method) which may cause unnecessarily creating encoder instances too. And the lifecycle is longer than using it as a local variable.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org