-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-19299. HttpReferrerAuditHeader resilience #7095
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-19299. HttpReferrerAuditHeader resilience #7095
Conversation
Catch all exceptions raised when building logger header log message at info; stack at debug Change-Id: I119abd167978dbc515554a36189462599870d1f9
|
🎊 +1 overall
This message was automatically generated. |
style checks + extra javadocs Change-Id: I1fbe381c49f51f9f4df62fd674a693c89ce9e9ed
|
tested s3 london. two failures from new tests which came with #7067 ; reopened that JIRA |
|
🎊 +1 overall
This message was automatically generated. |
...ct/hadoop-common/src/main/java/org/apache/hadoop/fs/store/audit/HttpReferrerAuditHeader.java
Show resolved
Hide resolved
| * @return a referrer string or "" | ||
| */ | ||
| public String buildHttpReferrer() { | ||
| public synchronized String buildHttpReferrer() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why need synchronized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the bug fix that has been found. Please see the jira for details. https://issues.apache.org/jira/browse/HADOOP-19299
mukund-thakur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 pending one log comment.
steveloughran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks; testing
...ct/hadoop-common/src/main/java/org/apache/hadoop/fs/store/audit/HttpReferrerAuditHeader.java
Show resolved
Hide resolved
Change-Id: If7a1ff24de449be395667f550341389a5083097c
|
tested s3 london; only failures were those of #7098 |
|
🎊 +1 overall
This message was automatically generated. |
|
Build happy; merging and backporting |
Let's no flood the logs if there is a recurrent problem, ok? Change-Id: I189a5b9cb7c6d264bc4835a631e3da5ec3bb22de
|
🎊 +1 overall
This message was automatically generated. |
* HttpReferrerAuditHeader is thread safe, copying the lists/maps passed in and using synchronized methods when necessary. * All exceptions raised when building referrer header are caught and swallowed. * The first such error is logged at warn, * all errors plus stack are logged at debug Contributed by Steve Loughran
* HttpReferrerAuditHeader is thread safe, copying the lists/maps passed in and using synchronized methods when necessary. * All exceptions raised when building referrer header are caught and swallowed. * The first such error is logged at warn, * all errors plus stack are logged at debug Contributed by Steve Loughran
HADOOP-19299
Catch all exceptions raised when building logger header log message at info; stack at debug
How was this patch tested?
new unit test
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?