Skip to content

HBASE-28334: Remove unnecessary null DEFAULT_VALUE #5662

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.

Sign up for GitHub

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

Merged
bbeaudreault merged 3 commits into from
Feb 5, 2024
Merged

HBASE-28334: Remove unnecessary null DEFAULT_VALUE #5662

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d79be3d
HBASE-28334: Remove unnecessary null DEFAULT_VALUE in TableDescriptor…
Feb 5, 2024
93ecf16
Updated Unit test comparision text
Feb 5, 2024
bd904b6
Reverting the change
Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ public class TableDescriptorBuilder {
private static final Bytes ERASURE_CODING_POLICY_KEY =
new Bytes(Bytes.toBytes(ERASURE_CODING_POLICY));

private static final String DEFAULT_ERASURE_CODING_POLICY = null;
/**
* Used by shell/rest interface to access this metadata attribute which denotes if the table
* should be treated by region normalizer.
Expand Down Expand Up @@ -234,7 +233,6 @@ public class TableDescriptorBuilder {
DEFAULT_VALUES.put(DURABILITY, DEFAULT_DURABLITY.name()); // use the enum name
DEFAULT_VALUES.put(REGION_REPLICATION, String.valueOf(DEFAULT_REGION_REPLICATION));
DEFAULT_VALUES.put(PRIORITY, String.valueOf(DEFAULT_PRIORITY));
DEFAULT_VALUES.put(ERASURE_CODING_POLICY, String.valueOf(DEFAULT_ERASURE_CODING_POLICY));
DEFAULT_VALUES.keySet().stream().map(s -> new Bytes(Bytes.toBytes(s)))
.forEach(RESERVED_KEYWORDS::add);
RESERVED_KEYWORDS.add(IS_META_KEY);
Expand Down
5 changes: 3 additions & 2 deletions hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,9 @@ public void testStringCustomizedValues() throws HBaseException {
.setMemStoreFlushSize("256MB").setErasureCodingPolicy("RS-6-3-1024k").build();
assertEquals(
"'testStringCustomizedValues', " + "{TABLE_ATTRIBUTES => {DURABILITY => 'ASYNC_WAL', "
+ "ERASURE_CODING_POLICY => 'RS-6-3-1024k', MAX_FILESIZE => '10737942528 B (10GB 512KB)', "
+ "MEMSTORE_FLUSHSIZE => '268435456 B (256MB)'}}, "
+ "MAX_FILESIZE => '10737942528 B (10GB 512KB)', "
+ "MEMSTORE_FLUSHSIZE => '268435456 B (256MB)', "
+ "METADATA => {'ERASURE_CODING_POLICY' => 'RS-6-3-1024k'}}}, "
Copy link
Contributor

@bbeaudreault bbeaudreault Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok, this is why we want to keep it in the defaults map. This setting should not be considered metadata. I couldn't remember this when I filled the jira.

Thank you for the PR. Now that I see this, I think we should keep it as it was. Maybe you can add a comment where it is added to the defaults map?

Copy link
Contributor Author

@nirdosh0110 nirdosh0110 Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bbeaudreault. Reverted the change and added a comment for future reference. Please Review

+ "{NAME => 'cf', BLOCKSIZE => '131072 B (128KB)'}",
htd.toStringCustomizedValues());
}
Expand Down