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

DSS-client C API update #93

Closed
wants to merge 3 commits into from
Closed

DSS-client C API update #93

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
11df089
DSS client C API update
Jan 11, 2024
5f7946c
Delete dss_s3benchmark/s3-benchmark
iso-p Jan 11, 2024
4c2cc78
strncpy safety
Jan 11, 2024
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
8 changes: 7 additions & 1 deletion dss_client/include/dss.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
#ifndef DSS_H
#define DSS_H

#define FAILURE -1
#define END_OF_LIST -2

#ifdef __cplusplus
extern "C" {
#endif
typedef void* DSSClient;
DSSClient DSSClientInit(char *ip, char* user, char* passwd, char* uuid, int endpoints_per_cluster);
int GetObjectBuffer(DSSClient c, void* key, int key_len, unsigned char* buffer, long int buffer_size);
int GetObject(DSSClient c, void* key, int key_len, char* dst_file);
int PutObject(DSSClient c, void* key, int key_len, char* src_file);
int PutObjectBuffer(DSSClient c, void* key, int key_len, unsigned char* buffer, long int content_length);
int DeleteObject(DSSClient c, void* key, int key_len);

int ListObjects(DSSClient c, char* prefix, char* delimit, char* obj_keys, int cur_pg);
int DeleteAll(DSSClient c, char* prefix, char* delimit);
int GetPageSize();
#ifdef __cplusplus
}
#endif
Expand Down
1 change: 1 addition & 0 deletions dss_client/include/dss_client.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ namespace dss {
uint32_t page_size = DSS_PAGINATION_DEFAULT);
std::set<std::string>&& ListObjects(const std::string& prefix, const std::string& delimiter);
std::set<std::string> ListBuckets();
std::unique_ptr<Objects> list_objs;

private:

Expand Down
100 changes: 100 additions & 0 deletions dss_client/src/dss_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,6 +1308,23 @@ namespace dss {
return ret;
}

extern "C" int PutObject(DSSClient c, void* key_name, int key_len, char* src_file)
{
Client *client = (Client*) c;
if (client == nullptr || (char*) key_name == nullptr){
return -1;
}
std::string key_str ((char*) key_name, key_len);
int ret = -1;

try {
ret = client->PutObject(key_str.c_str(), src_file);
} catch(...) {
printf("Exception caught in PutObject - %s\n", key_str.c_str());
}
return ret;
}

extern "C" int DeleteObject(DSSClient c, void* key_name, int key_len)
{
Client *client = (Client*) c;
Expand All @@ -1323,7 +1340,90 @@ namespace dss {
printf("Exception caught in DeleteObject for %s\n", key_str.c_str());
}
return ret;
}

extern "C" int ListObjects(DSSClient c, char* prefix, char* delimit, char* keys, int cur_pg)
{
Client *client = (Client*) c;
if (client == nullptr){
printf("the DSS client cannot be a null pointer.\n");
return FAILURE;
}
if (keys == nullptr){
printf("the buffer to store keys cannot be a null pointer.\n");
return FAILURE;
}
if (cur_pg == -1){ // for the first page of list
client->list_objs = client->GetObjects(prefix, delimit);
}
if (client->list_objs->GetObjKeys() < 0) { // reached the end of the pages for LIST
return END_OF_LIST;
}
std::string keys_concat = "";
for (std::string key: client->list_objs->GetPage()){
Copy link
Contributor

Choose a reason for hiding this comment

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

cpp + operator is very expensive when you have lot of objects. Rather than that, you can fall back to standard and efficient C based code. But this requires key_buffer_size to be passed as a variable

len = 0
for (key: list_objs->GetPage()) {
    len += snprintf(&keys[len],  key_buffer_size - len, "%s", keys.c_str());
    if (len ==key_buffer_size) {
        //Check the last keys.length() size in the key buffer to make sure the key is completely copied.
        //otherwise return error
    }
}

keys_concat += key + "\n"; // seperate each key by line assuming there is no "\n" in the object name or directory name or delimiter
}
if (keys_concat == "") return END_OF_LIST;
keys[keys_concat.length()] = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

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

Any possibility to get length of the key during the for loop at line 1363 itself?

Copy link
Author

Choose a reason for hiding this comment

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

yes, we can accumulate and get the sum of key lengths.

strncpy(keys, keys_concat.c_str(), keys_concat.length());
Copy link
Contributor

Choose a reason for hiding this comment

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

If above case doesn't work, you can store the keys_concat.length() in a variable and use that instead of computing length multiple times thereby introducing latency

if (keys[keys_concat.length()] != '\0') {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above for computing length
Is there any possibility to stop the keys_concat in the for loop itself if the memory allocation for keys is not sufficient

printf("the buffer overflow for storing keys\n");
return FAILURE;
}
return cur_pg + 1;
}

extern "C" int DeleteAll(DSSClient c, char* prefix, char* delimit)
{
Client *client = (Client*) c;
if (client == nullptr){
printf("the DSS client cannot be a null pointer.\n");
return FAILURE;
}
if (prefix == nullptr){
printf("the prefix cannot be a null pointer.\n");
return FAILURE;
}
if (delimit == nullptr){
printf("the delimiter cannot be a null pointer.\n");
return FAILURE;
}
uint32_t pg_size = DSS_PAGINATION_DEFAULT;
int max_key_len = 1024;
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to hash define this value (1024) as S3_MAX_KEY_SIZE_LIMIT at the top or in the header file for easy change in the future

char* keys = (char*) malloc(sizeof(char) * max_key_len * pg_size);
int pg;
while (true){
pg = ListObjects(client, prefix, delimit, keys, -1); // list the very first page
if (pg == FAILURE){
printf("ListObjects failed for prefix=\"%s\" and delimit=\"%s\".\n", prefix, delimit);
printf("DeleteAll failed for prefix=\"%s\" and delimit=\"%s\".\n", prefix, delimit);
free(keys);
return FAILURE;
}
if (pg == END_OF_LIST) break;
std::stringstream all_keys(keys);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can check whether strtok or stringstream is efficient and use it accordingly.

std::string each_key;
while (std::getline(all_keys, each_key, '\n')){ // retrieve each key by line
try {
if (client->DeleteObject(each_key.c_str()) < 0 ){ // delete the very first page listed
printf("DeleteObject failed for %s.\n", each_key.c_str());
free(keys);
return FAILURE;
}
// printf("Deleting %s\n", each_key.c_str());
} catch(...) {
printf("Exception caught in DeleteObject for %s.\n", each_key.c_str());
free(keys);
return FAILURE;
}
}
}
free(keys);
return 0;
}
extern "C" int GetPageSize() {
return DSS_PAGINATION_DEFAULT;
}

} // namespace dss

86 changes: 85 additions & 1 deletion dss_client/test/test_dss_c_interface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <fcntl.h>
#include <string.h>
#include <unistd.h>
#include <dirent.h>

#include "dss.h"

Expand All @@ -18,13 +19,55 @@ int hexdump(unsigned char* buff, int size)
return 0;
}

int putRecursive(DSSClient c, char *basePath)
{
int i;
char path[1000];
struct dirent *dp;
DIR *dir = opendir(basePath);

if (!dir){
return 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation needs to be fixed


while ((dp = readdir(dir)) != NULL)
{
if (strcmp(dp->d_name, ".") != 0 && strcmp(dp->d_name, "..") != 0)
{
strcpy(path, basePath);
strcat(path, "/");
strcat(path, dp->d_name);
// printf("%s\n", path);
FILE* fp = fopen(path, "r");
if (fp == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation issues

return 0;
}
fseek(fp, 0L, SEEK_END);
// calculating the size of the file
long int fsize = ftell(fp);
if (dp->d_type == DT_REG && fsize > 0){ // PutObject does not support empty files
if (PutObject(c, (void*) path, strlen(path), path) < 0 ){
Copy link
Contributor

Choose a reason for hiding this comment

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

fclose needs to be done

printf("PutObject - %s failed.\n", path);
return -1;
}
}
if (putRecursive(c, path) < 0) return -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

rather than going into the function putRecursive to check whether dir or not, you can add else case. Since you are already checking regular file in the top and you have dp->d_type handy.

}
}
closedir(dir);
return 0;
}

int main(int argc, char* argv[]) {
DSSClient *c;
int fd = -1;
int ret = -1;
int size = 1024*1024;
unsigned char* buff;
unsigned char* buff1;
int cur_page = -1;
int max_key_len = 1024;
Copy link
Contributor

Choose a reason for hiding this comment

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

use hash-define variable as mentioned above

char* keys_list;
char obj_name[256];
char uuid[] = "12345";

Expand All @@ -38,8 +81,9 @@ int main(int argc, char* argv[]) {
printf("Invalid endpoint URL\n");
return -1;
}
if (argc == 5)
if (argc >= 5)
Copy link
Contributor

Choose a reason for hiding this comment

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

why >= is required? If so, then the usage should be updated properly

size = atoi(argv[4]) * 1024;

buff = (unsigned char*)calloc(1, size);
buff1 = (unsigned char*)calloc(1, size);

Expand Down Expand Up @@ -77,10 +121,50 @@ int main(int argc, char* argv[]) {
ret = DeleteObject(c, (void*) obj_name, strlen(obj_name));
printf("Object testfile1 deleted\n");

// upload objects with a directory structure to test LIST
if (putRecursive(c, "/etc") < 0 ){
printf("Uploading directory failed.\n");
goto out;
}
//test the LIST
keys_list = (char*) malloc(sizeof(char) * max_key_len * GetPageSize());
Copy link
Contributor

Choose a reason for hiding this comment

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

since sizeof(char) is 1, you can see if this is really required. This may be applicable at the top as well.

if (keys_list == NULL) {
printf("malloc failed for allocating buffer to store the LIST results\n");
goto out;
}
while (1){
cur_page = ListObjects(c, "", "", keys_list, cur_page); // List all objects with prefix ""
if (cur_page == FAILURE){
printf("ListObjects failed.\n");
goto out;
}
if (cur_page == END_OF_LIST) break;
printf("page index = %d:\n[%s]\n", cur_page, keys_list);
}
//test the DeleteAll
ret = DeleteAll(c, "etc", "");
if (ret < 0) {
printf("DeleteAll failed.\n");
goto out;
}
//retest the LIST after DeleteAll
printf("After DeleteAll:\n");
cur_page = -1;
while (1){
cur_page = ListObjects(c, "", "", keys_list, cur_page); // List all objects with prefix=""
if (cur_page == FAILURE){
printf("ListObjects failed.\n");
goto out;
}
if (cur_page == END_OF_LIST) break;
printf("page index = %d:\n[%s]\n", cur_page, keys_list);
}

out:
if (fd != -1)
close(fd);
free(buff);
free(buff1);
free(keys_list);
return ret;
}