Skip to content

Commit

Permalink
Add config param vfs.file.enable_filelocks.
Browse files Browse the repository at this point in the history
Closes #1260.
  • Loading branch information
tdenniston committed May 29, 2019
1 parent 92b2566 commit 9159e6c
Show file tree
Hide file tree
Showing 12 changed files with 171 additions and 1 deletion.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
* Added datatype `tiledb_buffer_t` and functions `tiledb_buffer_{alloc,free,get_type,set_type,get_data,set_data}`.
* Added datatype `tiledb_buffer_list_t` and functions `tiledb_buffer_list_{alloc,free,get_num_buffers,get_total_size,get_buffer,flatten}`.
* Added string conversion functions `tiledb_*_to_str()` and `tiledb_*_from_str()` for all public enum types.
* Added config param `vfs.file.enable_filelocks`

### C++ API

Expand Down
2 changes: 2 additions & 0 deletions doc/source/tutorials/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,8 @@ along with their description and default values.
operations (any backend), per VFS instance.
``"vfs.file.max_parallel_ops"`` ``vfs.num_threads`` The maximum number of parallel operations on
objects with ``file:///`` URIs.
``"vfs.file.enable_filelocks"`` ``true`` If set to ``false``, file locking operations are
no-ops in VFS for ``file:///`` URIs.
``"vfs.min_batch_gap"`` ``"512000"`` The minimum number of bytes between two VFS
read batches.
``"vfs.min_batch_size"`` ``"20971520"`` The minimum number of bytes in a VFS
Expand Down
115 changes: 115 additions & 0 deletions test/src/unit-capi-array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1264,3 +1264,118 @@ TEST_CASE_METHOD(

remove_temp_dir(FILE_URI_PREFIX + FILE_TEMP_DIR);
}

TEST_CASE_METHOD(
ArrayFx,
"C API: Test array with no filelocks",
"[capi], [array], [array-no-filelocks]") {
std::string temp_dir;
if (supports_s3_)
temp_dir = S3_TEMP_DIR;
else if (supports_hdfs_)
temp_dir = HDFS_TEMP_DIR;
else
temp_dir = FILE_URI_PREFIX + FILE_TEMP_DIR;

std::string array_name = temp_dir + "array-no-filelocks";

// Create new TileDB context with file lock config disabled, rest the same.
tiledb_ctx_free(&ctx_);
tiledb_vfs_free(&vfs_);

tiledb_config_t* config = nullptr;
tiledb_error_t* error = nullptr;
REQUIRE(tiledb_config_alloc(&config, &error) == TILEDB_OK);
REQUIRE(error == nullptr);
REQUIRE(
tiledb_config_set(config, "vfs.file.enable_filelocks", "false", &error) ==
TILEDB_OK);
REQUIRE(error == nullptr);
if (supports_s3_) {
#ifndef TILEDB_TESTS_AWS_S3_CONFIG
REQUIRE(
tiledb_config_set(
config, "vfs.s3.endpoint_override", "localhost:9999", &error) ==
TILEDB_OK);
REQUIRE(
tiledb_config_set(config, "vfs.s3.scheme", "http", &error) ==
TILEDB_OK);
REQUIRE(
tiledb_config_set(
config, "vfs.s3.use_virtual_addressing", "false", &error) ==
TILEDB_OK);
REQUIRE(error == nullptr);
#endif
}
REQUIRE(tiledb_ctx_alloc(config, &ctx_) == TILEDB_OK);
REQUIRE(error == nullptr);
REQUIRE(tiledb_vfs_alloc(ctx_, config, &vfs_) == TILEDB_OK);
tiledb_config_free(&config);

create_temp_dir(temp_dir);

create_dense_vector(array_name);

// Prepare cell buffers
int buffer_a1[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
uint64_t buffer_a1_size = sizeof(buffer_a1);

// Open array
tiledb_array_t* array;
int rc = tiledb_array_alloc(ctx_, array_name.c_str(), &array);
CHECK(rc == TILEDB_OK);
rc = tiledb_array_open(ctx_, array, TILEDB_WRITE);
CHECK(rc == TILEDB_OK);

// Submit query
tiledb_query_t* query;
rc = tiledb_query_alloc(ctx_, array, TILEDB_WRITE, &query);
CHECK(rc == TILEDB_OK);
rc = tiledb_query_set_layout(ctx_, query, TILEDB_GLOBAL_ORDER);
CHECK(rc == TILEDB_OK);
rc = tiledb_query_set_buffer(ctx_, query, "a", buffer_a1, &buffer_a1_size);
CHECK(rc == TILEDB_OK);
rc = tiledb_query_submit(ctx_, query);
CHECK(rc == TILEDB_OK);
rc = tiledb_query_finalize(ctx_, query);
CHECK(rc == TILEDB_OK);

// Close array and clean up
rc = tiledb_array_close(ctx_, array);
CHECK(rc == TILEDB_OK);
tiledb_array_free(&array);
tiledb_query_free(&query);

int buffer_read[10];
uint64_t buffer_read_size = sizeof(buffer_read);

// Open array
rc = tiledb_array_alloc(ctx_, array_name.c_str(), &array);
CHECK(rc == TILEDB_OK);
rc = tiledb_array_open(ctx_, array, TILEDB_READ);
CHECK(rc == TILEDB_OK);

// Submit query
rc = tiledb_query_alloc(ctx_, array, TILEDB_READ, &query);
CHECK(rc == TILEDB_OK);
rc = tiledb_query_set_layout(ctx_, query, TILEDB_ROW_MAJOR);
CHECK(rc == TILEDB_OK);
rc =
tiledb_query_set_buffer(ctx_, query, "a", buffer_read, &buffer_read_size);
CHECK(rc == TILEDB_OK);
rc = tiledb_query_submit(ctx_, query);
CHECK(rc == TILEDB_OK);

// Close array and clean up
rc = tiledb_array_close(ctx_, array);
CHECK(rc == TILEDB_OK);
tiledb_array_free(&array);
tiledb_query_free(&query);

// Check correctness
int buffer_read_c[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
CHECK(!std::memcmp(buffer_read, buffer_read_c, sizeof(buffer_read_c)));
CHECK(buffer_read_size == sizeof(buffer_read_c));

remove_temp_dir(temp_dir);
}
3 changes: 3 additions & 0 deletions test/src/unit-capi-config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ void check_save_to_file() {
ss << "sm.num_tbb_threads -1\n";
ss << "sm.num_writer_threads 1\n";
ss << "sm.tile_cache_size 10000000\n";
ss << "vfs.file.enable_filelocks true\n";
ss << "vfs.file.max_parallel_ops " << std::thread::hardware_concurrency()
<< "\n";
ss << "vfs.min_batch_gap 512000\n";
Expand Down Expand Up @@ -395,6 +396,7 @@ TEST_CASE("C API: Test config iter", "[capi], [config]") {
all_param_values["vfs.min_parallel_size"] = "10485760";
all_param_values["vfs.file.max_parallel_ops"] =
std::to_string(std::thread::hardware_concurrency());
all_param_values["vfs.file.enable_filelocks"] = "true";
all_param_values["vfs.s3.scheme"] = "https";
all_param_values["vfs.s3.region"] = "us-east-1";
all_param_values["vfs.s3.aws_access_key_id"] = "";
Expand Down Expand Up @@ -426,6 +428,7 @@ TEST_CASE("C API: Test config iter", "[capi], [config]") {
vfs_param_values["min_parallel_size"] = "10485760";
vfs_param_values["file.max_parallel_ops"] =
std::to_string(std::thread::hardware_concurrency());
vfs_param_values["file.enable_filelocks"] = "true";
vfs_param_values["s3.scheme"] = "https";
vfs_param_values["s3.region"] = "us-east-1";
vfs_param_values["s3.aws_access_key_id"] = "";
Expand Down
2 changes: 1 addition & 1 deletion test/src/unit-cppapi-config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ TEST_CASE("C++ API: Config iterator", "[cppapi], [cppapi-config]") {
names.push_back(it->first);
}
// Check number of VFS params in default config object.
CHECK(names.size() == 26);
CHECK(names.size() == 27);
}
4 changes: 4 additions & 0 deletions tiledb/sm/c_api/tiledb.h
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,10 @@ TILEDB_EXPORT void tiledb_config_free(tiledb_config_t** config);
* The maximum number of parallel operations on objects with `file:///`
* URIs. <br>
* **Default**: `vfs.num_threads`
* - `vfs.file.enable_filelocks` <br>
* If set to `false`, file locking operations are no-ops for `file:///` URIs
* in VFS. <br>
* **Default**: `true`
* - `vfs.s3.region` <br>
* The S3 region, if S3 is enabled. <br>
* **Default**: us-east-1
Expand Down
4 changes: 4 additions & 0 deletions tiledb/sm/cpp_api/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,10 @@ class Config {
* The maximum number of parallel operations on objects with `file:///`
* URIs. <br>
* **Default**: `vfs.num_threads`
* - `vfs.file.enable_filelocks` <br>
* If set to `false`, file locking operations are no-ops for `file:///`
* URIs in VFS. <br>
* **Default**: `true`
* - `vfs.s3.region` <br>
* The S3 region, if S3 is enabled. <br>
* **Default**: us-east-1
Expand Down
6 changes: 6 additions & 0 deletions tiledb/sm/filesystem/vfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,9 @@ Status VFS::remove_file(const URI& uri) const {
Status VFS::filelock_lock(const URI& uri, filelock_t* fd, bool shared) const {
STATS_FUNC_IN(vfs_filelock_lock);

if (!vfs_params_.file_params_.enable_filelocks_)
return Status::Ok();

// Hold the lock while updating counts and performing the lock.
std::unique_lock<std::mutex> lck(filelock_mtx_);

Expand Down Expand Up @@ -395,6 +398,9 @@ Status VFS::filelock_lock(const URI& uri, filelock_t* fd, bool shared) const {
Status VFS::filelock_unlock(const URI& uri, filelock_t fd) const {
STATS_FUNC_IN(vfs_filelock_unlock);

if (!vfs_params_.file_params_.enable_filelocks_)
return Status::Ok();

// Hold the lock while updating counts and performing the unlock.
std::unique_lock<std::mutex> lck(filelock_mtx_);

Expand Down
3 changes: 3 additions & 0 deletions tiledb/sm/misc/constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,9 @@ const uint64_t vfs_min_batch_size = 20 * 1024 * 1024;
/** The default maximum number of parallel file:/// operations. */
const uint64_t vfs_file_max_parallel_ops = vfs_num_threads;

/** Whether or not filelocks are enabled for VFS. */
const bool vfs_file_enable_filelocks = true;

/** The maximum name length. */
const uint32_t uri_max_len = 256;

Expand Down
3 changes: 3 additions & 0 deletions tiledb/sm/misc/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,9 @@ extern const uint64_t vfs_min_batch_size;
/** The default maximum number of parallel file:/// operations. */
extern const uint64_t vfs_file_max_parallel_ops;

/** Whether or not filelocks are enabled for VFS. */
extern const bool vfs_file_enable_filelocks;

/** The maximum name length. */
extern const uint32_t uri_max_len;

Expand Down
20 changes: 20 additions & 0 deletions tiledb/sm/storage_manager/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ Status Config::set(const std::string& param, const std::string& value) {
RETURN_NOT_OK(set_vfs_min_batch_size(value));
} else if (param == "vfs.file.max_parallel_ops") {
RETURN_NOT_OK(set_vfs_file_max_parallel_ops(value));
} else if (param == "vfs.file.enable_filelocks") {
RETURN_NOT_OK(set_vfs_file_enable_filelocks(value));
} else if (param == "vfs.s3.region") {
RETURN_NOT_OK(set_vfs_s3_region(value));
} else if (param == "vfs.s3.aws_access_key_id") {
Expand Down Expand Up @@ -429,6 +431,12 @@ Status Config::unset(const std::string& param) {
value << vfs_params_.file_params_.max_parallel_ops_;
param_values_["vfs.file.max_parallel_ops"] = value.str();
value.str(std::string());
} else if (param == "vfs.file.enable_filelocks") {
vfs_params_.file_params_.enable_filelocks_ =
constants::vfs_file_enable_filelocks;
value << (vfs_params_.file_params_.enable_filelocks_ ? "true" : "false");
param_values_["vfs.file.enable_filelocks"] = value.str();
value.str(std::string());
} else if (param == "vfs.s3.region") {
vfs_params_.s3_params_.region_ = constants::s3_region;
value << vfs_params_.s3_params_.region_;
Expand Down Expand Up @@ -652,6 +660,10 @@ void Config::set_default_param_values() {
param_values_["vfs.file.max_parallel_ops"] = value.str();
value.str(std::string());

value << (vfs_params_.file_params_.enable_filelocks_ ? "true" : "false");
param_values_["vfs.file.enable_filelocks"] = value.str();
value.str(std::string());

value << vfs_params_.s3_params_.region_;
param_values_["vfs.s3.region"] = value.str();
value.str(std::string());
Expand Down Expand Up @@ -962,6 +974,14 @@ Status Config::set_vfs_file_max_parallel_ops(const std::string& value) {
return Status::Ok();
}

Status Config::set_vfs_file_enable_filelocks(const std::string& value) {
bool v;
RETURN_NOT_OK(utils::parse::convert(value, &v));
vfs_params_.file_params_.enable_filelocks_ = v;

return Status::Ok();
}

Status Config::set_vfs_s3_region(const std::string& value) {
vfs_params_.s3_params_.region_ = value;
return Status::Ok();
Expand Down
9 changes: 9 additions & 0 deletions tiledb/sm/storage_manager/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,11 @@ class Config {

struct FileParams {
uint64_t max_parallel_ops_;
bool enable_filelocks_;

FileParams() {
max_parallel_ops_ = constants::vfs_file_max_parallel_ops;
enable_filelocks_ = constants::vfs_file_enable_filelocks;
}
};

Expand Down Expand Up @@ -335,6 +337,10 @@ class Config {
* The maximum number of parallel operations on objects with `file:///`
* URIs. <br>
* **Default**: `vfs.num_threads`
* - `vfs.file.enable_filelocks` <br>
* If set to `false`, file locking operations are no-ops for `file:///`
* URIs in VFS. <br>
* **Default**: `true`
* - `vfs.s3.region` <br>
* The S3 region, if S3 is enabled. <br>
* **Default**: us-east-1
Expand Down Expand Up @@ -568,6 +574,9 @@ class Config {
/** Sets the max number of allowed file:/// parallel operations. */
Status set_vfs_file_max_parallel_ops(const std::string& value);

/** Sets the file locking enabled state. */
Status set_vfs_file_enable_filelocks(const std::string& value);

/** Sets the S3 region. */
Status set_vfs_s3_region(const std::string& value);

Expand Down

0 comments on commit 9159e6c

Please sign in to comment.