[ https://issues.apache.org/jira/browse/CASSANDRA-15234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17162209#comment-17162209 ]
Ekaterina Dimitrova commented on CASSANDRA-15234: ------------------------------------------------- Good catch [~benedict] . Thank you. My bad, just pushed a fast commit [here|https://github.com/ekaterinadimitrova2/cassandra/commit/b4eebe080835da79d032f9314262c268b71172a8] for the three rate parameters we have. For the record, I stopped working on this patch until it is clear whether the code will be used at all. As far as I understand, you took the lead on a POC development for the newly suggested approach. Unfortunately, I don't have time to support this now but I would be happy to give feedback when it is shaped already. If you decide at some point this work or part of it to be committed, let me know, I will complete whatever is outstanding. The main framework is in place. A couple of things to keep in mind: * As suggested by Benjamin, the patch can be simplified by making {{Converter}} an {{enum}}. It will allow to use the enum value directly into the {{Replaces}} annotation. Doing so will remove the need to use reflection to instantiate the converters and the use of a cache to avoid multiple instantiation. * In-jvm tests - loading config parameters should be reworked as currently they don't work with custom types (like the newly introduced Duration, etc). The suggested approach by [~dcapwell] would work but it requires also api changes. I suggest a separate ticket for this part to be opened. Instead of using reflection, the suggestion is to use SnakeYAML. In order not to slow down the tests, no yaml files will be introduced but there will be a function to build the yaml nodes for us. This was a quick POC by [~dcapwell] but there are parameters which will need additional work and attention: public class MapToConfigTest { @Test public void test() { Map<String, Object> map = ImmutableMap.<String, Object>builder() .put("auto_bootstrap", false) .put("permissions_validity_in_ms", 10) .put("role_manager", "some value") .build(); Constructor constructor = new YamlConfigurationLoader.CustomConstructor(Config.class); PropertiesChecker propertiesChecker = new PropertiesChecker(); constructor.setPropertyUtils(propertiesChecker); constructor.setComposer(new Composer(null, null) { public Node getSingleNode() { return toNode(map); } }); Config config = (Config) constructor.getSingleData(Config.class); System.out.println("trap"); } public static Node toNode(Object object) { if (object instanceof Map) { List<NodeTuple> values = new ArrayList<>(); for (Map.Entry<Object, Object> e : ((Map<Object, Object>) object).entrySet()) { values.add(new NodeTuple(toNode(e.getKey()), toNode(e.getValue()))); } return new MappingNode(FAKE_TAG, values, null); } else if (object instanceof Number || object instanceof String || object instanceof Boolean) { return new ScalarNode(FAKE_TAG, object.toString(), FAKE_MARK, FAKE_MARK, '\''); } else { throw new UnsupportedOperationException("unexpected type found: given " + object.getClass()); } } private static final Tag FAKE_TAG = new Tag("ignore"); private static final Mark FAKE_MARK = new Mark("ignore", 0, 0, 0, "", 0); } The rest are small things which I also partially already handled. [~maedhroz], [~benedict], please, let me know if you have any other questions around this patch. > Standardise config and JVM parameters > ------------------------------------- > > Key: CASSANDRA-15234 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15234 > Project: Cassandra > Issue Type: Bug > Components: Local/Config > Reporter: Benedict Elliott Smith > Assignee: Ekaterina Dimitrova > Priority: Normal > Fix For: 4.0-beta > > Attachments: CASSANDRA-15234-3-DTests-JAVA8.txt > > > We have a bunch of inconsistent names and config patterns in the codebase, > both from the yams and JVM properties. It would be nice to standardise the > naming (such as otc_ vs internode_) as well as the provision of values with > units - while maintaining perpetual backwards compatibility with the old > parameter names, of course. > For temporal units, I would propose parsing strings with suffixes of: > {{code}} > u|micros(econds?)? > ms|millis(econds?)? > s(econds?)? > m(inutes?)? > h(ours?)? > d(ays?)? > mo(nths?)? > {{code}} > For rate units, I would propose parsing any of the standard {{B/s, KiB/s, > MiB/s, GiB/s, TiB/s}}. > Perhaps for avoiding ambiguity we could not accept bauds {{bs, Mbps}} or > powers of 1000 such as {{KB/s}}, given these are regularly used for either > their old or new definition e.g. {{KiB/s}}, or we could support them and > simply log the value in bytes/s. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org