Skip to content
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.

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

Add mounts_points to stats #739

Closed

Conversation

dmitrizagidulin
Copy link

Add a {mount_points, [...]} section to the stats. This is a list of file system mount points relevant to Riak (specifically, location of platform_data_dir), for use with disk_stats() to determine Riak disk space usage.

Currently, the {disk, [...]} section of stats lists all of the mount points on the system and their usage. However, there is no way to programmatically tell which of those Riak (the back-ends, specifically) is mounted on.
This PR adds that missing component, to enable the disk usage calculation needed by basho/riak_cs#612

Add a ```{mount_points, [...]}``` section to the stats list
%% @doc Get a list of filesystem mount points relevant to Riak.
%% Currently includes the mount point for the riak data directory (platform_data_dir).
%% For use with disk_stats() to determine Riak disk space usage.
-spec mount_point_stats() -> proplist().
Copy link
Member

Choose a reason for hiding this comment

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

I get

/Users/russell/dev/e/basho/pre6/riak/deps/riak_kv/src/riak_kv_stat_bc.erl:363: type proplist() undefined
ERROR: compile failed while processing /Users/russell/dev/e/basho/pre6/riak/deps/riak_kv: rebar_abort

When compiling.

proplist:proplist() works for me

@russelldb
Copy link
Member

Seems to work. I'm surprised to see that when I run riak-admin status my mount point is "/"

Also, when I get /stats the mount point is

 "mount_points": {
    "platform_data_dir": [
      47
    ]
  },

I guess a victim of strings are lists of integers

@slfritchie
Copy link
Contributor

Dmitri, the 2.0 code freeze is approaching. Will you have time to fix this PR soon?

@dmitrizagidulin
Copy link
Author

Ah, yes, thanks! Will fix it today.

On Thu, Dec 12, 2013 at 7:15 AM, Scott Lystig Fritchie <
notifications@github.com> wrote:

Dmitri, the 2.0 code freeze is approaching. Will you have time to fix this
PR soon?


Reply to this email directly or view it on GitHubhttps://github.com//pull/739#issuecomment-30415128
.

@kuenishi
Copy link
Contributor

For example, elevedb actually uses the configuration set as data_root .

 %% eLevelDB Config
 {eleveldb, [
             {data_root, "./data/leveldb"}
            ]},

To know the actual disk (or mem) usage, we'd better set the responsibility to answer the proper space left, into each riak_kv_backend as a new behaviour callback like this:

-callback space_left() ->
    {ok, {SpaceLeftBytes::non_neg_integer(),
             SpaceLeftPercentage::0..100 }}
  | {error, any()}

The implementation of each would be based on either data_root+diskup:get_disk_data(), just a memory or else.

Also because, Riak CS uses riak_cs_kv_multi_backend 's special data directory as block and object kv spaces. So just pulling platform_data_dir won't work for Riak CS.

To make Riak CS work, for now we need app.config in Riak 1.4 be like this:

{storage_backend, riak_cs_kv_multi_backend},
{multi_backend_prefix_list, [{<<"0b:">>, be_blocks}]},
{multi_backend_default, be_default},
{multi_backend, [
        {be_default, riak_kv_eleveldb_backend, [
            {max_open_files, 50},
            {data_root, "/home/kuenishi/staging2/data/leveldb"}
                                      ]},
        {be_blocks, riak_kv_bitcask_backend, [
             {data_root, "/home/kuenishi/staging2/data/bitcask"}
                                      ]}
     ]},

@kuenishi
Copy link
Contributor

basho/riak_cs#723 also needs this with a way to calculste total space left all over a cluster (which would be another issue).

@dmitrizagidulin
Copy link
Author

@kuenishi I agree with your points about data_root and the additional complications that Riak CS's multi_backend presents.
However, keep in mind, that the code in this PR ( dmitrizagidulin@6f4a602 ) calculates storage on the level of disk partitions, not directories. Meaning, it only requires that the various backend data dirs live on the same disk partition. They can be in separate directories, no problem.

While it may be possible that, in the future, the space left calculation is best left to the various riak_kv_backends, I do think that to start with, simply introducing the mount_points section to the /stats output is a helpful first step.

@dmitrizagidulin
Copy link
Author

@kuenishi perhaps we should add a list of the data_root values for each backend, to the mount points section?

@reiddraper
Copy link
Contributor

-callback space_left() ->
    {ok, {SpaceLeftBytes::non_neg_integer(),
             SpaceLeftPercentage::0..100 }}
  | {error, any()}

I think just returning UsedBytes and TotalBytes is more flexible than calculating that percentage in this code. The consumer can choose to calculate a percent if that's what they want.

@dmitrizagidulin
Copy link
Author

@reiddraper Two questions, then.

  1. So.. what's the upshot, in terms of this PR? Scrap it, and open issues on each backend to implement the space_left() call?

  2. From an implementation standpoint, how are the backends going to calculate space left? Part of the reason that I ended up calculating space left on a per-disk-partition basis (and not on a per-directory basis) is because it's quite expensive to calculate (on the OS level) the disk space taken up by a directory (especially while it's under load and constantly changing). Whereas disk-partition size is cached/tracked by the OS, and is much cheaper. So my question is -- do the backends currently keep tabs on the total size taken up? Or will they have to iterate over objects & perform the calculations (or cache/roll them up as they go)?
    [Edit: Ah, never mind, I see that part of @kuenishi's implementation proposal mentions that the backends can calculate usage using a similar method as this PR (diskup:get_disk_data() plus a shell call to figure out which partition that backend's data_root resides).]

@reiddraper
Copy link
Contributor

That's probably best answered by someone on the core/kv cabal.

@jtuple jtuple added this to the 2.1 milestone Mar 24, 2014
@martincox martincox closed this Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants