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.

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

Jump to bottom

integrate open source expiry features into eleveldb option processing #203

Merged
merged 3 commits into from
Jul 6, 2016
Merged

integrate open source expiry features into eleveldb option processing #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72b6ced
integrate open source expiry features into eleveldb option processing
Jun 29, 2016
bb316af
temporarily point to leveldb tag expiry_beta1
Jul 6, 2016
2f58433
set build_deps.sh to develop branch
Jul 6, 2016
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
53 changes: 53 additions & 0 deletions c_src/eleveldb.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "leveldb/perf_count.h"
#define LEVELDB_PLATFORM_POSIX
#include "util/hot_threads.h"
#include "leveldb_os/expiry_os.h"

#ifndef INCL_WORKITEMS_H
#include "workitems.h"
Expand Down Expand Up @@ -133,6 +134,9 @@ ERL_NIF_TERM ATOM_TIERED_SLOW_LEVEL;
ERL_NIF_TERM ATOM_TIERED_FAST_PREFIX;
ERL_NIF_TERM ATOM_TIERED_SLOW_PREFIX;
ERL_NIF_TERM ATOM_CACHE_OBJECT_WARMING;
ERL_NIF_TERM ATOM_EXPIRY_ENABLED;
ERL_NIF_TERM ATOM_EXPIRY_MINUTES;
ERL_NIF_TERM ATOM_WHOLE_FILE_EXPIRY;
} // namespace eleveldb


Expand Down Expand Up @@ -217,6 +221,8 @@ class eleveldb_priv_data
public:
EleveldbOptions m_Opts;
leveldb::HotThreadPool thread_pool;
leveldb::ExpiryPtr_t m_ExpiryModule; // only populated if expiry options seen
// (self deleting pointer)

explicit eleveldb_priv_data(EleveldbOptions & Options)
: m_Opts(Options),
Expand Down Expand Up @@ -459,6 +465,45 @@ ERL_NIF_TERM parse_open_option(ErlNifEnv* env, ERL_NIF_TERM item, leveldb::Optio
opts.cache_object_warming = false;
}

else if (option[0] == eleveldb::ATOM_EXPIRY_ENABLED)
{
if (option[1] == eleveldb::ATOM_TRUE)
{
if (NULL==opts.expiry_module.get())
Copy link
Contributor

Choose a reason for hiding this comment

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

Should checking/creating the expiry_module member be moved outside the ATOM_TRUE conditional? That way, we set the expiry_enabled member, regardless of whether the expiry_module member was previously created. Then we don't have to worry about the order in which the various expiry options are declared or what their defaults are in the leveldb::ExpiryModuleOS class.

The same question/comment applies to the other expiry options below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Keep in mind that we do not know what order options will arrive. And that cuttlefish often passes default values to common options.

I am intentionally not creating the object unless absolutely necessary. It is highly likely that the default options will get passed ... and defaults currently leave expiry disabled. So only creating the object if non-defaults seen. However, I do update the object if a different non-default option caused its creation.

opts.expiry_module.assign(new leveldb::ExpiryModuleOS);
((leveldb::ExpiryModuleOS *)opts.expiry_module.get())->expiry_enabled = true;
} // if
else
{
if (NULL!=opts.expiry_module.get())
((leveldb::ExpiryModuleOS *)opts.expiry_module.get())->expiry_enabled = false;
} // else
} // else if
else if (option[0] == eleveldb::ATOM_EXPIRY_MINUTES)
{
unsigned long minutes(0);
if (enif_get_ulong(env, option[1], &minutes))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we log (syslog?) an error if the enif_Xxx function fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created issue to track suggestion for future. This is of same nature as your suggestion to have pthread creation errors post to syslog. I think I will do all of these at once.

#204

{
if (NULL==opts.expiry_module.get())
opts.expiry_module.assign(new leveldb::ExpiryModuleOS);
((leveldb::ExpiryModuleOS *)opts.expiry_module.get())->expiry_minutes = minutes;
} // if
} // else if
else if (option[0] == eleveldb::ATOM_WHOLE_FILE_EXPIRY)
{
if (option[1] == eleveldb::ATOM_TRUE)
{
if (NULL==opts.expiry_module.get())
opts.expiry_module.assign(new leveldb::ExpiryModuleOS);
((leveldb::ExpiryModuleOS *)opts.expiry_module.get())->whole_file_expiry = true;
} // if
else
{
if (NULL!=opts.expiry_module.get())
((leveldb::ExpiryModuleOS *)opts.expiry_module.get())->whole_file_expiry = false;
} // else
} // else if

}

return eleveldb::ATOM_OK;
Expand Down Expand Up @@ -568,8 +613,13 @@ async_open(
eleveldb_priv_data& priv = *static_cast<eleveldb_priv_data *>(enif_priv_data(env));

leveldb::Options *opts = new leveldb::Options;
opts->expiry_module.assign(priv.m_ExpiryModule.get());

fold(env, argv[2], parse_open_option, *opts);

opts->fadvise_willneed = priv.m_Opts.m_FadviseWillNeed;
if (NULL==priv.m_ExpiryModule.get() && NULL!=opts->expiry_module.get())
Copy link
Contributor

Choose a reason for hiding this comment

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

I am unclear on the purpose of this code. A few lines above, you assigned opts->expiry_module from priv.m_ExpiryModule, so they should be the same. If there's some reason why they may not be the same, a comment here would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Line 616: has a previous vnode opened and created an expiry module? Give me the previously created object if it exists.
Line 621: if there was no previously created expiry module and one suddenly appears due to this fold, make it the default.

This squirrel code defends against the future possibility of having different expiry modules by vnode and yet still a global default. Not likely ... but possible given that the future code is not yet written.

priv.m_ExpiryModule.assign(opts->expiry_module.get());

// convert total_leveldb_mem to byte count if it arrived as percent
// This happens now because there is no guarantee as to when the total_memory
Expand Down Expand Up @@ -1282,6 +1332,9 @@ try
ATOM(eleveldb::ATOM_TIERED_FAST_PREFIX, "tiered_fast_prefix");
ATOM(eleveldb::ATOM_TIERED_SLOW_PREFIX, "tiered_slow_prefix");
ATOM(eleveldb::ATOM_CACHE_OBJECT_WARMING, "cache_object_warming");
ATOM(eleveldb::ATOM_EXPIRY_ENABLED, "expiry_enabled");
ATOM(eleveldb::ATOM_EXPIRY_MINUTES, "expiry_minutes");
ATOM(eleveldb::ATOM_WHOLE_FILE_EXPIRY, "whole_file_expiry");
#undef ATOM


Expand Down
3 changes: 2 additions & 1 deletion c_src/refobjects.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,12 +415,13 @@ LevelIteratorWrapper::LogIterator()
struct tm created;

localtime_r(&m_IteratorCreated, &created);
#if 0 // available in different branch
leveldb::Log(m_DbPtr->m_Db->GetLogger(),
"Iterator created %d/%d/%d %d:%d:%d, move operations %zd (%p)",
created.tm_mon, created.tm_mday, created.tm_year-100,
created.tm_hour, created.tm_min, created.tm_sec,
m_MoveCount, m_Iterator);

#endif
} // LevelIteratorWrapper::LogIterator()


Expand Down
28 changes: 28 additions & 0 deletions priv/eleveldb.schema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,31 @@
{datatype, {enum, [true, false]}},
hidden
]}.

%% @doc Enable global expiry. All leveldb databases / vnodes
%% will use same expiry_minutes and whole_file_expiry settings.
%% Same settings apply to all leveldb instances in multi_backend.
{mapping, "leveldb.expiry_enabled", "eleveldb.expiry_enabled", [
{default, off},
{datatype, flag},
hidden
]}.

%% @doc Minutes until object expires. Gives number of minutes
%% a stored key/value will stay within the database before automatic
%% deletion. Zero disables expiry by age (minutes) feature
{mapping, "leveldb.expiry_minutes", "eleveldb.expiry_minutes", [
{default, 0},
{datatype, integer},
Copy link
Contributor

Choose a reason for hiding this comment

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

When you parse the expiry_minutes option in eleveldb.cc, you use the enif_get_ulong() function. Is that compatible with declaring it as an "integer" here? I'm not sure of the type system in this schema, which is why I'm asking (e.g., perhaps integers are always long and unsigned).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cuttlefish is its own little world. I do copy/paste of previous options as my base. The enif_get_XxX() functions used in eleveldb.cc are defined as filtering out of range values. Therefore my defense is at that layer, not here or other weirdness in eleveldb.erl.

hidden
]}.

%% @doc Expire entire .sst table file. Authorizes leveldb to
%% eliminate entire files that contain expired data (delete files
%% instead of removing expired key/values during compaction).
{mapping, "leveldb.whole_file_expiry", "eleveldb.whole_file_expiry", [
{default, off},
{datatype, flag},
hidden
]}.

21 changes: 21 additions & 0 deletions priv/eleveldb_multi.schema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,24 @@
hidden
]}.

%% @see leveldb.expiry_enabled
{mapping, "multi_backend.$name.leveldbexpiry_enabled", "riak_kv.multi_backend", [
{default, off},
{datatype, flag},
hidden
]}.

%% @see leveldb.expiry_minutes
{mapping, "multi_backend.$name.leveldbexpiry_minutes", "riak_kv.multi_backend", [
{default, 0},
{datatype, integer},
hidden
]}.

%% @see leveldb.whole_file_expiry
{mapping, "multi_backend.$name.leveldbwhole_file_expiry", "riak_kv.multi_backend", [
{default, off},
{datatype, flag},
hidden
]}.

11 changes: 9 additions & 2 deletions src/eleveldb.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ init() ->
{tiered_slow_level, pos_integer()} |
{tiered_fast_prefix, string()} |
{tiered_slow_prefix, string()} |
{cache_object_warming, boolean()}].
{cache_object_warming, boolean()} |
{expiry_enabled, boolean()} |
{expiry_minutes, pos_integer()} |
{whole_file_expiry, boolean()}
].

-type read_option() :: {verify_checksums, boolean()} |
{fill_cache, boolean()} |
Expand Down Expand Up @@ -307,7 +311,10 @@ option_types(open) ->
{tiered_slow_level, integer},
{tiered_fast_prefix, any},
{tiered_slow_prefix, any},
{cache_object_warming, bool}];
{cache_object_warming, bool},
{expiry_enabled, bool},
{expiry_minutes, integer},
{whole_file_expiry, bool}];

option_types(read) ->
[{verify_checksums, bool},
Expand Down