nfsantos commented on code in PR #1645:
URL: https://github.com/apache/jackrabbit-oak/pull/1645#discussion_r1718038932
##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneDocumentMaker.java:
##########
@@ -300,18 +300,42 @@ protected boolean indexTypeOrderedFields(Document doc,
String pname, int tag, Pr
fieldAdded = true;
} else {
long now = System.currentTimeMillis();
- if (now > lastDuplicateWarning +
DUPLICATE_WARNING_INTERVAL_MS) {
+ if (now > lastOrderedPropertyWarning +
ORDERED_PROPERTY_WARNING_INTERVAL_MS) {
log.warn("Duplicate value for ordered field {};
ignoring. Possibly duplicate index definition.", f.name());
- lastDuplicateWarning = now;
+ lastOrderedPropertyWarning = now;
}
}
}
} catch (Exception e) {
- log.warn(
- "[{}] Ignoring ordered property. Could not convert
property {} of type {} to type {} for path {}",
- getIndexName(), pname,
- Type.fromTag(property.getType().tag(), false),
- Type.fromTag(tag, false), path, e);
+ boolean unknownWarning = true;
+ String message = e.getMessage();
+ if (message.startsWith("Not a date string") ||
+ message.startsWith("Unable to parse the provided date
field") ||
+ message.startsWith("For input string")) {
Review Comment:
I don't agree that we should suppress altogether some types of errors.
Either the error message is important and should always be logged (although
removing duplicates), or else it is not important and it should not be logged
at all. But a probabilistic approach where the message is logged in some runs
but not in other runs, depending only on chance does not seem to be correct.
From what I saw, the LogSilencer is thread-safe and supports bounding the
memory usage, so there is not risk of OOME.
https://github.com/apache/jackrabbit-oak/blob/396b3c4626cec75d35638146ee9aab335e8a387d/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/LogSilencer.java#L34-L45
The only thing I don't like so much about the LogSilencer is that in this
case it would be more useful to count the number of times a message is
suppressed and then print a warning like:
> <ErrorMessage> (this message was repeated n times).
So that we would know how widespread is the particular issue (if a few dozen
times in a run, then ignore, if it appears millions of times, may be worth to
investigate).
--
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]
