-
Notifications
You must be signed in to change notification settings - Fork 526
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
Memcache Support #2096
Memcache Support #2096
Conversation
e5a62fe
to
81efaa0
Compare
676140c
to
359e7ca
Compare
recheck |
e7512e8
to
dc4fb31
Compare
@@ -59,7 +60,7 @@ list: | |||
@bash util/build.sh --stor=$(stor) --list | |||
|
|||
build: | |||
@bash util/build.sh --stor=$(stor) --only=$(only) --dep=$(dep) --release=$(release) --os=$(os) | |||
@bash util/build.sh --stor=${stor} --only=$(only) --dep=$(dep) --release=$(release) --ci=$(ci) --os=$(os) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wthat's the 'ci' option used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!mdsClient_->AllocOrGetMemcacheCluster(fsInfo_->fsid(), | ||
&kvcachecluster)) { | ||
LOG(ERROR) << "FLAGS_supportKVcache = " << FLAGS_supportKVcache | ||
<< ", but AllocOrGetMemcacheCluster fail"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more space before "fail"
return false; | ||
} | ||
|
||
// std::string ip[3] = {"10.182.2.46", "10.182.2.47", "10.182.2.48"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete code needn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
} | ||
|
||
bool KVClientManager::Get(std::shared_ptr<GetKvCacheContext> task) { | ||
LatencyGuard guard(&g_kvClientMetric->kvClientGet.latency); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 'Get' function can request the 'Get' function in line84.
assert(nullptr != task->value);
return Get(task->key, task->value, task->offset, task->length);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
bool MdsClientImpl::AllocOrGetMemcacheCluster( | ||
uint32_t fsId, curvefs::mds::topology::MemcacheCluster *cluster) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use a fake one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will be implemented by #2108
GenerateKVReuqest(inodeWrapper, memCacheMissRequest, dataBuf, | ||
&kvRequest); | ||
|
||
// read from s3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read from diskcache or s3?
The read order is kvcache -> diskcache -> s3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localcache/remote kv cluster fail will not return error code.
Failure to read from s3 will eventually return failure.
done.
// meaningful error code | ||
// read from s3 not exist | ||
// 1. may be the metaserver compaction update inode is not | ||
// synchronized to the client. clear inodecache && get agin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. ReadKVRequest is already handled with while.
The situations of s3 return error will retry:If the error is not exist, it may be that the data fragment has not been transmitted to s3, and it will wait for retry according to HandleReadS3NotExist
. Other errors indicate that reading from s3 failed and will not be retried.
copy_file ./thirdparties/aws/aws-sdk-cpp/build/aws-cpp-sdk-s3-crt/libaws-cpp-sdk-s3-crt.so $docker_prefix | ||
copy_file ./thirdparties/memcache/libmemcached-1.1.2/build-libmemcached/src/libmemcached/libmemcached.so $docker_prefix | ||
copy_file ./thirdparties/memcache/libmemcached-1.1.2/build-libmemcached/src/libmemcached/libmemcached.so.11 $docker_prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lrwxrwxrwx 1 xxxx 18 Dec 7 14:05 libmemcached.so -> libmemcached.so.11
lrwxrwxrwx 1 xxxx 22 Dec 7 14:05 libmemcached.so.11 -> libmemcached.so.11.0.0
-rwxr-xr-x 1 xxxx 225728 Dec 7 14:05 libmemcached.so.11.0.0
need copy libmemcached.so.11.0.0 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the perspective of ldd, it directly depends on so.11, and the actual construction environment test only needs to rely on so.11.
return false; | ||
} | ||
|
||
// std::string ip[3] = {"10.182.2.46", "10.182.2.47", "10.182.2.48"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete unused code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
} | ||
|
||
// init kvcacheclient manager | ||
// KVClientManagerOpt config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete unused code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
bool MdsClientImpl::AllocOrGetMemcacheCluster( | ||
uint32_t fsId, curvefs::mds::topology::MemcacheCluster *cluster) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return false derectly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will be implemented by #2108
#define ONRETURN(TYPE, RES, KEY, ERRORLOG) \ | ||
if (RES) { \ | ||
g_kvClientMetric->kvClient##TYPE.qps.count << 1; \ | ||
VLOG(9) << "Set key = " << KEY << " OK"; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set key -> type key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
VLOG(9) << "Set key = " << KEY << " OK"; \ | ||
} else { \ | ||
g_kvClientMetric->kvClient##TYPE.eps.count << 1; \ | ||
LOG(ERROR) << "Set key = " << KEY << " error = " << ERRORLOG; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set key -> type key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
char *databuf, uint64_t offset, | ||
uint64_t length, int *ret) { | ||
uint64_t start = butil::cpuwide_time_us(); | ||
*ret = s3ClientAdaptor_->GetS3Client()->Download(name, databuf, offset, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is only the logic of sync download here, do you not consider async download?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's a remaining problem and is fixed by #2080
@@ -80,6 +80,11 @@ struct SpaceAllocServerOption { | |||
uint64_t rpcTimeoutMs; | |||
}; | |||
|
|||
struct KVClientManagerOpt { | |||
int setThreadPooln = 4; | |||
int getThreadPooln = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getThreadPooln where does this param use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a reserved parameter, libmemcached, the current overall reading process is synchronous. Also waiting for pr #2080
dc4fb31
to
f347a8f
Compare
2bdaf12
to
f5a0193
Compare
Signed-off-by: ilixiaocui <[email protected]>
f5a0193
to
ba71cc1
Compare
What problem does this PR solve?
Issue Number: #xxx
Problem Summary:
What is changed and how it works?
What's Changed:
How it Works:
Side effects(Breaking backward compatibility? Performance regression?):
Check List