[
https://issues.apache.org/jira/browse/HADOOP-19993?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18119080#comment-18119080
]
ASF GitHub Bot commented on HADOOP-19993:
-----------------------------------------
joseluisll commented on PR #8753:
URL: https://github.com/apache/hadoop/pull/8753#issuecomment-5829474336
Thanks for picking this up. I reproduced the warning locally and I think the
patch is right in approach but over-applied by one field: as written it swaps
`SE_BAD_FIELD` for `SE_TRANSIENT_FIELD_NOT_RESTORED`, so `hadoop-common` still
reports one extant warning and trunk precommit stays red.
Three runs on a clean branch off trunk at `90f0d1da37c`, same command each
time, reading `hadoop-common-project/hadoop-common/target/spotbugsXml.xml` (not
just the exit code):
```
./mvnw -pl hadoop-common-project/hadoop-common -DskipTests -P'!native-win'
test-compile spotbugs:spotbugs
```
**1. Baseline, unmodified trunk — `total_bugs = 1`**
```
type='SE_BAD_FIELD' priority='2' rank='16' category='BAD_PRACTICE'
Class: org.apache.hadoop.mcp.McpHttpServlet
Field: name='requestHandler'
signature='Lorg/apache/hadoop/mcp/McpRequestHandler;'
Class org.apache.hadoop.mcp.McpHttpServlet defines non-transient
non-serializable instance field requestHandler
```
**2. This PR's diff (both fields `transient`) — `total_bugs = 1`**
```
type='SE_TRANSIENT_FIELD_NOT_RESTORED' priority='2' rank='16'
category='BAD_PRACTICE'
Class: org.apache.hadoop.mcp.McpHttpServlet
Field: name='objectMapper'
signature='Lcom/fasterxml/jackson/databind/ObjectMapper;'
The field org.apache.hadoop.mcp.McpHttpServlet.objectMapper is transient but
isn't set by deserialization
```
`SE_BAD_FIELD` is indeed gone, but the new warning lands on `objectMapper`.
**3. Only `requestHandler` marked `transient` — `total_bugs = 0`**
Clean. (`total_classes='2648'` on all three runs; `McpHttpServlet` is in the
analyzed set each time.)
The reason for the asymmetry: Jackson's `ObjectMapper` is itself
`Serializable`, so it never tripped `SE_BAD_FIELD` — which is why trunk reports
one warning rather than two. Marking it `transient` is what introduces
`SE_TRANSIENT_FIELD_NOT_RESTORED`. `McpRequestHandler` is not `Serializable`,
so `transient` there is the sanctioned fix and draws no complaint. Both fields
being `final` turned out not to matter to either detector.
So the suggestion is just to drop the `objectMapper` hunk and keep:
```java
private final ObjectMapper objectMapper;
private final transient McpRequestHandler requestHandler;
```
That also matches what other hadoop-common servlets do —
`ProfileServlet.process` and `JMXJsonServlet.mBeanServer` / `jsonFactory` use
`transient` for exactly the non-serializable collaborators and leave the rest
alone.
Happy to be wrong if your run shows something different — worth confirming,
since the Jenkins run on this PR only checks that the *patch* is clean, not
that the module reaches zero.
> Fix SpotBugs SE_BAD_FIELD in McpHttpServlet blocking trunk precommit
> --------------------------------------------------------------------
>
> Key: HADOOP-19993
> URL: https://issues.apache.org/jira/browse/HADOOP-19993
> Project: Hadoop Common
> Issue Type: Bug
> Components: common
> Reporter: Wei-Chiu Chuang
> Priority: Major
> Labels: pull-request-available
>
> After YARN-11977 added the MCP HTTP server in hadoop-common, SpotBugs reports
> one remaining warning on trunk in hadoop-common-project/hadoop-common:
> * SE_BAD_FIELD: Class org.apache.hadoop.mcp.McpHttpServlet defines
> non-transient non-serializable instance field requestHandler
> Yetus runs with spotbugs-strict-precheck. While this warning exists on trunk,
> PRs that build modules depending on hadoop-common can fail precommit with:
> {code}hadoop-common-project/hadoop-common in trunk has 1 extant spotbugs
> warnings.{code}
> even when patch SpotBugs and unit tests pass (example: PR-8744 / HDFS-17981).
> *Fix:* mark servlet dependency fields transient (HttpServlet is Serializable
> but instances are not serialized in normal use).
> *Pull request:* https://github.com/apache/hadoop/pull/8753
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
