liuxiaocs7 commented on code in PR #8840:
URL: https://github.com/apache/hbase/pull/8840#discussion_r4467525540
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheUtil.java:
##########
@@ -59,30 +59,30 @@ public class BlockCacheUtil {
/**
* Needed generating JSON.
Review Comment:
nit: could we note why `TRANSIENT` and `STATIC` are listed? They look
redundant because they're Gson's defaults, which is how they got dropped in
HBASE-29325. A short comment should stop that from happening again:
```suggestion
* Needed generating JSON. {@code excludeFieldsWithModifiers} replaces
Gson's default exclusion
* of static and transient fields rather than adding to it, so they have
to be listed explicitly.
* Otherwise cache internals such as {@code BucketCache.backingMap} end up
in the JSON.
```
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/CacheTestUtils.java:
##########
@@ -378,11 +384,23 @@ public static void getBlockAndAssertEquals(BlockCache
cache, BlockCacheKey key,
}
public static void testConvertToJSON(BlockCache toBeTested) {
+ String json = null;
Review Comment:
nit: `assertDoesNotThrow` would remove the need for the `null`
initialization and the try/catch:
```java
String json = assertDoesNotThrow(() -> BlockCacheUtil.toJSON(toBeTested));
assertNotNull(json);
```
--
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]
