client: remove dsn::apps::mutate in client.h #177
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qinzuoyan
reviewed
Sep 10, 2018
src/include/pegasus/client.h
Outdated
| std::vector<::dsn::apps::mutate> _mu_list; | ||
| std::vector<std::pair<int, int>> _ttl_list; | ||
| std::vector<mutate> mu_list; | ||
| std::vector<std::pair<int, int>> ttl_list; |
src/include/pegasus/client.h
Outdated
| int current_time = ::pegasus::utils::epoch_now(); | ||
| mutations = _mu_list; | ||
| for (auto &pair : _ttl_list) { | ||
| int current_time = time(nullptr) - epoch_begin; |
There was a problem hiding this comment.
这里还是用::pegasus::utils::epoch_now()吧，不要在include里面定义epoch_begin，避免暴露实现。
mutation的set/del/get_mutations实现体都比较大，可以在client_lib中增加一个cpp文件，把实现放在cpp中间中，这样也能避免client.h引用peagsus_utils头文件。
added 2 commits
September 10, 2018 18:13
qinzuoyan
reviewed
Sep 10, 2018
| @@ -0,0 +1,68 @@ | |||
| #include <pegasus/client.h> | |||
| mutations[pair.first].set_expire_ts_seconds = pair.second + current_time; | ||
| } | ||
| } | ||
| } |
| for (int i = 0; i < mutate_list.size(); ++i) { | ||
| auto &mu = mutate_list[i]; | ||
| req.mutate_list[i].operation = (dsn::apps::mutate_operation::type)mu.operation; | ||
| req.mutate_list[i].sort_key.assign(mu.sort_key.c_str(), 0, mu.sort_key.size()); |
There was a problem hiding this comment.
这里有问题：mutate_list是个局部变量，将其assign给blob时，数据是浅拷贝。在这个异步函数结束的时候，mutate_list被释放，req里面的blob就有可能是野指针。建议这里使用blob::create_from_bytes函数，既避免重复的数据拷贝，又避免野指针。譬如：
req.mutate_list[i].sort_key = blob::create_from_bytes(std::move(mu.sort_key));
req.mutate_list[i].value = blob::create_from_bytes(std::move(mu.value));
src/include/pegasus/client.h
Outdated
| mutate_operation operation; | ||
| std::string sort_key; | ||
| std::string value; | ||
| int set_expire_ts_seconds; |
src/include/pegasus/client.h
Outdated
| std::string sort_key; | ||
| std::string value; | ||
| int set_expire_ts_seconds; | ||
| }; |
added 2 commits
September 10, 2018 21:01
qinzuoyan
reviewed
Sep 11, 2018
src/client_lib/mutation.cpp
Outdated
| { | ||
| mutate mu; | ||
| mu.operation = mutate::mutate_operation::MO_DELETE; | ||
| mu.sort_key = sort_key; |
There was a problem hiding this comment.
mu.sort_key = std::move(sort_key);
qinzuoyan
reviewed
Sep 11, 2018
src/client_lib/mutation.cpp
Outdated
| { | ||
| mutate mu; | ||
| mu.operation = mutate::mutate_operation::MO_DELETE; | ||
| mu.sort_key = std::move(sort_key); |
qinzuoyan
approved these changes
Sep 11, 2018
hycdong
approved these changes
Sep 11, 2018
neverchanje
pushed a commit
that referenced
this pull request
Sep 14, 2018
vagetablechicken
pushed a commit
to vagetablechicken/pegasus
that referenced
this pull request
Sep 28, 2018
* client: remove dsn::apps::mutate in client.h (apache#177) * server: incr support reset ttl (apache#178) * update rdsn (apache#179) * scripts: build without building rdsn tests (apache#182) * submodule: use new random APIs of rdsn (apache#183)
neverchanje
pushed a commit
to neverchanje/pegasus
that referenced
this pull request
Jul 13, 2019
Former-commit-id: 9be860eb902656bce116db8e2d3128aefd56b7fa [formerly 92c1f57] Former-commit-id: 6a9bb9b3656f89d887d0ef1c285f2168fe39d1c9
cauchy1988
added a commit
to cauchy1988/incubator-pegasus
that referenced
this pull request
May 7, 2022
acelyc111
pushed a commit
that referenced
this pull request
Jun 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.