Add option 'participate_in_2i_coverage' with default 'enabled'

[russelldb]

This option is processed in riak_core when gathering nodes to
participate in coverage queries.

See basho/riak_core#917 for details. This PR just adds the schema mapping for that feature

[ghost]

Shouldn't this be getting added to riak_core ?

[russelldb]

Sadly that's not how it works

[binarytemple]

I'm confused, can you clarify please

[russelldb]

But I haven't thought much about it. Should it even have 2i in the name? Is it a general, riak_core, participate in coverage option?

I do think there is not the abstraction/separation that people expect from core. But we did agree that basho/riak_core was for riak_kv in the last meeting.

[ghost]

This p/r is just for the cuttlefish schema - all the related p/r are targeted at riak_core - this p/r is targeted at riak_kv - seems strange.

[russelldb]

I think that's just how it is with the fish. Look at the existing schema, search riak_core.

[russelldb]

And it makes sense, riak_kv uses riak_core, so riak_kv sets the application env variables for riak_core based on its use. If you had a typical erlang release with a sys.config that depends on 10 apps, where do you configure your release? In the top level sys.config, not each dependant app, right?

[ghost]

Hmm. It doesn't appear to be that way with every other project in the tree. The schema is placed in the same project as the functionality it controls - then they are ordered and concatenated at release time.

/basho/riak develop-2.2*

⚛︎□▷  find ./deps | grep schema   
./deps/riak_kv/test/riak_kv_schema_tests.erl
./deps/riak_kv/priv/riak_kv.schema
./deps/riak_kv/priv/multi_backend.schema
./deps/yokozuna/test/yokozuna_schema_tests.erl
./deps/yokozuna/priv/yokozuna.schema
./deps/riak_control/test/riak_control_schema_test.erl
./deps/riak_control/priv/riak_control.schema
./deps/riak_repl/test/riak_repl_schema_tests.erl
./deps/riak_repl/priv/riak_repl.schema
./deps/riak_sysmon/test/riak_sysmon_schema_tests.erl
./deps/riak_sysmon/priv/riak_sysmon.schema
./deps/eleveldb/test/eleveldb_schema_tests.erl
./deps/eleveldb/test/multi_backend.schema
./deps/eleveldb/priv/eleveldb_multi.schema
./deps/eleveldb/priv/eleveldb.schema
./deps/riak_api/test/riak_api_schema_tests.erl
./deps/riak_api/priv/riak_api.schema
./deps/riak_core/test/riak_core_schema_tests.erl
./deps/riak_core/priv/riak_core.schema
./deps/bitcask/test/bitcask_schema_tests.erl
./deps/bitcask/priv/bitcask_multi.schema
./deps/bitcask/priv/bitcask.schema

[russelldb]

I disagree. There are schemas for projects and then there is config at the top-level. It can be both. Did you read my example above? Did you look at the kv schema?

Let's get down to it: are you saying add this config to riak_core schema in order for there to be a +1 on this feature?

[ghost]

To my mind it would seem the place to put it. Perhaps get a second opinion from someone like @martincox ? Apart from that, it's +1.

[russelldb]

I don't get it. Should we also move:

https://github.com/ramensen/riak_kv/blob/abea4e0a7b4799760ad48df3519d697b5b7837da/priv/riak_kv.schema#L430-L556 ?

And the riak_dt compression option https://github.com/ramensen/riak_kv/blob/abea4e0a7b4799760ad48df3519d697b5b7837da/priv/riak_kv.schema#L430-L556?

Let me explain again, using riak_dt compression as an example. Riak_dt the app exposes the configuration option for compression (it may even have a default in riak_dt), but riak_kv (the USER of the dependancy) decides the value. For riak_kv that value is 1. For other applications (LASP, AntidoteDB, whatever) that value might be 3 or 0 or whatever, and so they would set that in their sys.config or schema. Does that make sense?

[ghost]

In this case - the user sets the configuration value in riak.conf:

participate_in_coverage=disabled 

(on reflection 'true' or 'false' would have been nicer there but whatevs)

And riak_core performs a read of that value, here:

https://github.com/basho/riak_core/pull/917/files#diff-187256fc99c333fb811ba3c5edae7f00R516

and here:

https://github.com/basho/riak_core/pull/917/files#diff-79f7a4d38797ea467d8ef98f44d1a0d3R162

Riak KV doesn't really come into it - apart from the targeting of this schema change at 'riak_kv' repository - that's why I made the suggestion.

I looked through the examples you gave, yep, the mixing of riak_core settings into the kv schema has been going on for 4 years now - I can't say that's great imho.

The way cuttlefish was intended to be used was, define the setting in the project (a) where the value is used, override it in project b which uses a.

From the wiki

reltool.config

You'll need to tell reltool.config about any schema files you want included. You'll also need to tell reltool.config the priority of those files. Add them to your {overlay, [...]} section, like this:

%% Cuttlefish Schema Files have a priority order.
%% Anything in a file prefixed with 00- will override
%% anything in a file with a higher numbered prefix.
{template, "files/riak.schema", "lib/00-riak.schema"},
{template, "../deps/<dependency_name>/priv/<schema_name>.schema", "lib/02-.schema"},
{template, "files/vm.args", "etc/vm.args"},

The priority is defined as "alphabetical order". which means, if the file is named A.schema, everything in it overrides things in B.schema if there's a conflict. As the reltool.config author, you can determine schema priority by renaming schema files in the template tuple.

• Why would you do this? Say we provided a riak_core.schema (and we plan to!), say you like all our setting names for your riak_core application, except one. You could override that one in your own schema! Freedom! Choices! Options! See what I do for you?*

Does this make sense to you or am I going off on a tangent ?

[russelldb]

You're on a tangent. Riak_KV comes into it because riak_kv has 2i queries. It uses the config. Not riak_core. I am also a strong disagree on true | false vs enabled | disabled as are the inaka best practices doc, and anyone who prefers semantic atoms vs. general booleans

[russelldb]

I know how the fish works and how it builds the config, but the configuration of THIS riak_core property is not a matter for riak_core, it is a matter for riak_kv. I don't know how better to explain it than to say (again) that the application that uses the dependancy configures the dependancy for it's own use.

[russelldb]

Describing the setting of kv defaults for core as " the mixing of riak_core settings into the kv schema " is just incorrect. KV uses core, so it configures core. Right? We don't ship core configured for KV, the applications that build on core configure for their use cases.

[russelldb]

@bryanhuntesl it doesn't scale, nor is it fair on him, to call coxy in to arbitrate. You took the review on, I'd like to know if my arguments above have made any sense to you.

Another example I thought of…imagine you write a product that sits on top of riak, let's call it riak_cs (for sake of argument). Riak exposes certain configurable options (backend, n_val etc). Your product, riak_cs, sets these things, it doesn't expect Riak to define values for riak_cs. Sure, riak defines defaults, but riak_cs configures riak (backend=multi_backend etc) for it's use case. Right?

[russelldb]

If nothing else, it is a least idiomatic to configure the setting this way, you must concede that

[ghost]

Your arguments don't make sense to me.

I've used cuttlefish many times with nested projects. The schema goes in the project which it is used to configure, you then override the defaults (if required) at release time and via the riak.conf file. It's always been that way.

Regarding the two examples you gave, their configuration is in the wrong place, I recall once, during my Basho tenure, trying to discover what a cuttlefish setting was being converted to but being unable to locate it, it was frustrating at the time, I was trying to solve a customer issue.

Because someone has done something wrong once, does that become the norm? It's not the end of the world, but no, it is not idiomatic or correct to put the schema configuration for one project into another.

Cuttlefish is not just a convenience utility, it provides some documentation for a projects environment variables, tweaks and settings.

I provided an excerpt and a link to the cuttlefish wiki in my earlier comment.

I don't have anything more to add, if you want to get someone else to take a look, I'm happy to accept their judgement.

[russelldb]

So what do you need to happen? The config should be in kv, but you want a default in the core schema too?

[russelldb]

We need some consistency here. Someone from bet365 just yolo merges adding riak_repl to riak, but this…this change…

[martinsumner]

@bryanhuntesl - will you accept me as an arbitrator on this? I agree with @russelldb, but even if I didn't, I don't think there is a new issue being introduced which justifies holding everything up.

The GitHub issue will hold your counter-argument and opinion for the permanent record, and given the bike-shed nature of this, I think that should be enough.

Can we +1?

[ghost]

No problem - your vote is the decider. +1