Skip to content

Loading…

Az655 osync warning #175

Merged
merged 4 commits into from

3 participants

@massung

Requires branch az655-osync-warning from riak_core as well. On linux, checks for bitcask config {sync_strategy,o_sync} and warns (if on Linux) that it will fail.

@rzezeski rzezeski commented on an outdated diff
src/riak_kv_bitcask_backend.erl
@@ -89,6 +89,7 @@ start(Partition, Config) ->
BitcaskOpts = [{read_write, true} | Config],
case bitcask:open(filename:join(DataRoot, DataFile), BitcaskOpts) of
Ref when is_reference(Ref) ->
+ check_fcntl(),
@rzezeski
rzezeski added a note

Substitute hard tabs for spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jonmeredith jonmeredith commented on an outdated diff
src/riak_kv_bitcask_backend.erl
@@ -309,6 +310,25 @@ key_counts(RootDir) ->
%% ===================================================================
%% @private
+%% On linux there is a kernel bug that won't allow fcntl to add O_SYNC
+%% to an already open file descriptor.
+check_fcntl() ->
+ Logged=application:get_env(riak_kv,o_sync_warning_logged),
+ Strategy=application:get_env(bitcask,sync_strategy),
+ case {Logged,Strategy} of
+ {undefined,{ok,o_sync}} ->
+ case riak_core_util:is_arch(linux) of
+ true ->
+ lager:warning("{sync_strategy,o_sync} fails on Linux"),
@jonmeredith Basho Technologies member

I would say 'not implemented' rather than fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rzezeski

So why not put this check in riak_kv_app:start/2 instead and avoid the "check if I already logged this" code?

@jonmeredith
Basho Technologies member

Because it's a pluggable backend. It's in the right place.

@rzezeski

+1 to merge

@massung massung merged commit 6a29591 into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 20 additions and 0 deletions.
  1. +20 −0 src/riak_kv_bitcask_backend.erl
View
20 src/riak_kv_bitcask_backend.erl
@@ -89,6 +89,7 @@ start(Partition, Config) ->
BitcaskOpts = [{read_write, true} | Config],
case bitcask:open(filename:join(DataRoot, DataFile), BitcaskOpts) of
Ref when is_reference(Ref) ->
+ check_fcntl(),
schedule_merge(Ref),
maybe_schedule_sync(Ref),
{ok, #state{ref=Ref,
@@ -309,6 +310,25 @@ key_counts(RootDir) ->
%% ===================================================================
%% @private
+%% On linux there is a kernel bug that won't allow fcntl to add O_SYNC
+%% to an already open file descriptor.
+check_fcntl() ->
+ Logged=application:get_env(riak_kv,o_sync_warning_logged),
+ Strategy=application:get_env(bitcask,sync_strategy),
+ case {Logged,Strategy} of
+ {undefined,{ok,o_sync}} ->
+ case riak_core_util:is_arch(linux) of
+ true ->
+ lager:warning("{sync_strategy,o_sync} not implemented on Linux"),
+ application:set_env(riak_kv,o_sync_warning_logged,true);
+ _ ->
+ ok
+ end;
+ _ ->
+ ok
+ end.
+
+%% @private
%% Return a function to fold over the buckets on this backend
fold_buckets_fun(FoldBucketsFun) ->
fun(#bitcask_entry{key=BK}, {Acc, BucketSet}) ->
Something went wrong with that request. Please try again.