You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "torwig (via GitHub)" <gi...@apache.org> on 2023/04/27 20:01:50 UTC
[GitHub] [incubator-kvrocks] torwig opened a new pull request, #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
torwig opened a new pull request, #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405
Currently, Redis Stream has 3 strategies to form the new entry's ID (provided with the XADD command):
- autogenerated: the `*` pattern
- fully specified by the client: the `12345-6789` pattern
- only the sequence number is auto-generated, and the timestamp is specified by the client: the `12345-*` pattern.
This PR introduces an additional pattern `*-12345` when a client is able to specify a sequence number and a timestamp will be generated by the Kvrocks server.
The new pattern allows clients to use their internal IDs as part of an entry ID.
Kvrocks generates timestamps and order incoming entries by the receiving time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "torwig (via GitHub)" <gi...@apache.org>.
torwig commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180648284
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
(*res)->MethodsInDerivedClass(); // or `std::unique_ptr x = std::move(*res);`, whatever
} else {
// print or forward (return) `res.Msg()`, or `return res` directly
}
```
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
(*res)->MethodsInDerivedClass(); // or `std::unique_ptr x = std::move(*res);`, whatever
} else {
// print or forward `res.Msg()`, or `return res` directly
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>` instead of exceptions, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
(*res)->AnyMethod(); // or `std::unique_ptr x = std::move(*res);`, whatever
} else {
// print or forward `res.Msg()`, or `return res` directly
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>` instead of exceptions, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
(*res)->MethodsInDerivedClass(); // or `std::unique_ptr x = std::move(*res);`, whatever
} else {
// print or forward `res.Msg()`, or `return res` directly
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
} else {
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] aleksraiden commented on pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#issuecomment-1526732160
And some of futures - this feature will be pretty use with XMOVE/XCOPY next command planned too... ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice merged pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID via XADD
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
(*res)->MethodsInDerivedClass(); // or `std::unique_ptr x = std::move(*res);`, whatever
} else {
print res.Msg();
}
```
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
(*res)->MethodsInDerivedClass(); // or `std::unique_ptr x = std::move(*res);`, whatever
} else {
print or forward (return) res.Msg();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] aleksraiden commented on pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#issuecomment-1526722566
Awesome feature, we are waiting for this, lot of thanks! For example, we can use this streamID for checking latency for each message (if consumer after processing, store an result with a same ID, but new autogenerated part of time).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID…
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#discussion_r1180405528
##########
src/types/redis_stream_base.cc:
##########
@@ -85,39 +79,48 @@ Status ParseStreamEntryID(const std::string &input, StreamEntryID *id) {
return Status::OK();
}
-Status ParseNewStreamEntryID(const std::string &input, NewStreamEntryID *id) {
+std::unique_ptr<NextStreamEntryIDGenerationStrategy> ParseNextStreamEntryIDStrategy(const std::string &input) {
Review Comment:
From my side, I recommend you to use something like `StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>>`, e.g.
```c++
StatusOr<std::unique_ptr<NextStreamEntryIDGenerationStrategy>> ParseNextStreamEntryIDStrategy(const std::string &input) {
...
if (...) {
return {Status::NotOk, "some reason..."};
} else {
return std::make_unique<SomeDerivedClass>(...);
}
...
}
```
And use it as usual:
```c++
auto res = ParseNextStreamEntryIDStrategy(...);
if (res.IsOK()) {
(*res)->MethodsInDerivedClass(); // or `std::unique_ptr x = std::move(*res);`, whatever
} else {
// print or forward (return) `res.Msg()`;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #1405: Add the ability to use the '*-123' pattern to specify stream entry ID via XADD
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #1405:
URL: https://github.com/apache/incubator-kvrocks/pull/1405#issuecomment-1528581452
Merging...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org