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.

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

server: support check_and_set #122

Merged
merged 5 commits into from
Jul 17, 2018
Merged

server: support check_and_set #122

merged 5 commits into from
Jul 17, 2018

Conversation

qinzuoyan
Copy link
Contributor

No description provided.

@neverchanje
Copy link
Contributor

  1. 这个功能是为了解决现有什么需求的?

  2. 我们只需要 checkAndSet 吗?如果以后需要 checkAndDelete 或者 checkAndMultiSet 如何扩展?我建议可以借鉴 hbase 的 checkAndMutate 接口,这个接口更通用。 http://hbase.apache.org/apidocs/src-html/org/apache/hadoop/hbase/client/Table.html#line.477

@qinzuoyan
Copy link
Contributor Author

  1. 因为wifi万能钥匙那边有需求。另外check_and_set是比较基本的原子操作,以后做transaction等都需要依赖这个功能。

  2. 现在确实只支持set一个值。你说的checkAndMutate我也考虑过,不过接口会复杂一些,我准备在以后有需求的时候一起加,同时还增加直接的mutate(不check)。在Mutation中可以放入多个set和del操作,因为现在的multiSet和multiDel无法同时set和del。

  3. 所以现在这个版本算是一个基础版吧。mutate因为对客户端接口改动较大,以后有需求再加。

req.check_sort_key.assign(check_sort_key.c_str(), 0, check_sort_key.size());
req.check_type = (dsn::apps::cas_check_type::type)check_type;
req.check_oprand.assign(check_oprand.c_str(), 0, check_oprand.size());
if (check_sort_key != set_sort_key) {
Copy link
Member

Choose a reason for hiding this comment

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

相同的时候就不用给req.set_sort_key赋值吗?

sed 's/#include "rrdb_types.h"/#include <rrdb\/rrdb_types.h>/' $TMP_DIR/rrdb_types.cpp > ../base/rrdb_types.cpp

rm -rf $TMP_DIR

echo
echo "You should manually modify these files:"
Copy link
Member

Choose a reason for hiding this comment

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

没看懂这个是什么意思?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

因为这三个文件的代码都是手写的(有些特殊的定制规则),所以不能使用thrift生成的文件。

@@ -99,6 +99,48 @@ class pegasus_client
}
};

enum cas_check_type
Copy link
Member

Choose a reason for hiding this comment

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

这个也可以直接使用thrift里面生成的吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我们的Pegasus客户端头文件要尽量少依赖,所以不要依赖thrift的头文件

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

case ::dsn::apps::cas_check_type::CT_VALUE_BYTES_GREATER: {
if (!value_exist)
return false;
int c = dsn::string_view(value).compare(dsn::string_view(check_operand));
Copy link
Contributor

Choose a reason for hiding this comment

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

我们的binary_compare是不是也可以顺手删了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// return true if check passed.
// for int compare, if check operand or value are not valid integer, then return false,
// and set out param `invalid_argument' to false.
bool validate_check(int64_t decree,
Copy link
Contributor

Choose a reason for hiding this comment

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

这种函数能不能单独抽出来写单元测试,而不是通过集成测试的方式来做。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

构造context比较麻烦,现在的测试已经比较全面了

@@ -40,24 +40,102 @@ inline bool version(command_executor *e, shell_context *sc, arguments args)
return true;
}

#define NAME_EQUAL(name) ((len = strlen(name)) == length && strncmp(buffer, name, len) == 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

统一用string view的compare?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

inline bool
buf2cas_check_type(const char *buffer, int length, pegasus::pegasus_client::cas_check_type &result)
{
int len = 0;
Copy link
Contributor

@shengofsun shengofsun Jul 17, 2018

Choose a reason for hiding this comment

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

这个地方可以用enum_helper吗?或者commond_utils.h下的type_from_string有没有帮助?这么写略琐碎

Copy link
Contributor Author

Choose a reason for hiding this comment

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

不能,还是那个问题,pegasus client头文件依赖要简单,不要依赖rdsn的头文件。

Copy link
Contributor

@shengofsun shengofsun Jul 17, 2018

Choose a reason for hiding this comment

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

commands.h不用担心依赖的问题的

@qinzuoyan qinzuoyan merged commit 4c8e64f into master Jul 17, 2018
@qinzuoyan qinzuoyan deleted the qinzuoyan2 branch July 17, 2018 11:31
@neverchanje neverchanje added component/api pegasus client interfaces and removed new feature labels Sep 13, 2018
neverchanje pushed a commit to neverchanje/pegasus that referenced this pull request Jul 13, 2019
Former-commit-id: b7b7ec6b9134d4ac0acdde7971d975f5adee22e0 [formerly 4c8e64f]
Former-commit-id: 18d00bce2280b67135699e4c55b91c3b25b5feaa
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
Labels
component/api pegasus client interfaces
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants