davidradl commented on code in PR #27499:
URL: https://github.com/apache/flink/pull/27499#discussion_r2753594835
##########
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java:
##########
@@ -188,7 +248,17 @@ void checkAvroInitialized() throws IOException {
if (datumReader != null) {
return;
}
-
+ synchronized (this) {
+ if (isFastReaderEnabled()) {
+ String openFlag = System.getProperty(FAST_READER_PROP);
+ if (StringUtils.isEmpty(openFlag)) {
+ System.setProperty(FAST_READER_PROP, "true");
+ LOG.info(
+ "{} are enabled, but FAST_READER_PROP is empty. We
just change it to true.",
Review Comment:
The `System.setProperty(FAST_READER_PROP, "true"); `is process wide so we
effect all threads- it does not seem right that one thread for one
deserialization should change something process wide. Is there a case that some
threads would not want this on? Are there any downsides to specifying this
option?
##########
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java:
##########
@@ -188,7 +248,17 @@ void checkAvroInitialized() throws IOException {
if (datumReader != null) {
return;
}
-
+ synchronized (this) {
+ if (isFastReaderEnabled()) {
+ String openFlag = System.getProperty(FAST_READER_PROP);
+ if (StringUtils.isEmpty(openFlag)) {
+ System.setProperty(FAST_READER_PROP, "true");
+ LOG.info(
+ "{} are enabled, but FAST_READER_PROP is empty. We
just change it to true.",
Review Comment:
I find this message confusing. `if FAST_READER_PROP is empty,` the test
implies the code changes the system property to true. I suggest
"{} is enabled, and will be honoured, even though FAST_READER_PROP is
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]
