honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix handling of fractional byte size value settings (#37172) 

This commit addresses an issue when setting a byte size value setting
using a value that has a fractional component when converted to its
string representation. For example, trying to set a byte size value
setting to a value of 1536 bytes is problematic because internally this
is converted to the string "1.5k". When we go to get this setting, we
try to parse "1.5k" back to a byte size value, which does not support
fractional values. The problem is that internally we are relying on a
method which loses the unit when doing the string conversion. Instead,
we are going to use a method that does not lose the unit and therefore
we can roundtrip from the byte size value to the string and back to the
byte size value.
This commit is contained in:
Jason Tedor 2019-01-07 07:13:50 -08:00 committed by GitHub
parent ad4e4fd9b8
commit a233db7367
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
server/src/main/java/org/elasticsearch/common/settings/Settings.java
View File

@ -861,8 +861,8 @@ public final class Settings implements ToXContentFragment {
* @param byteSizeValue The setting value
* @return The builder
*/
public Builder put(String key, ByteSizeValue byteSizeValue) {
return put(key, byteSizeValue.toString());
public Builder put(final String key, final ByteSizeValue byteSizeValue) {
return put(key, byteSizeValue.getStringRep());
}
/**

18
server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java
View File

@ -25,6 +25,8 @@ import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
@ -726,4 +728,20 @@ public class SettingsTests extends ESTestCase {
assertThat(actual, equalTo(expected));
}
public void testFractionalByteSizeValue() {
final Setting<ByteSizeValue> setting =
Setting.byteSizeSetting("key", ByteSizeValue.parseBytesSizeValue(randomIntBetween(1, 16) + "k", "key"));
final ByteSizeValue expected = new ByteSizeValue(randomNonNegativeLong(), ByteSizeUnit.BYTES);
final Settings settings = Settings.builder().put("key", expected).build();
/*
* Previously we would internally convert the byte size value to a string using a method that tries to be smart about the units
* (e.g., 1024 bytes would be converted to 1kb). However, this had a problem in that, for example, 1536 bytes would be converted to
* 1.5k. Then, 1.5k could not be converted back to a ByteSizeValue because ByteSizeValues do not support fractional components.
* Effectively this test is then asserting that we no longer make this mistake when doing the internal string conversion. Instead,
* we convert to a string using a method that does not lose the original unit.
*/
final ByteSizeValue actual = setting.get(settings);
assertThat(actual, equalTo(expected));
}
}