You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/09/16 15:34:46 UTC
[GitHub] [couchdb] nickva opened a new pull request, #4179: Don't double-encode changes sequence strings in the replicator
nickva opened a new pull request, #4179:
URL: https://github.com/apache/couchdb/pull/4179
Previously we always encoded the sequence as json. In case of a strings, the most common case, this ended up with something like "\"1-gA..\"". Then, the endpoints would strip out and ignore the extra `"`. So, avoid sending the extra bytes just so they can striped out in the end anyway. This should make the logs look a bit cleaner too.
For integers or any other sequences we still json encode them. So we `1` will be sent `since=1` parameter. And, in the unlikely we case we replicate with a BigCouch 0.4 era endpoint, the `[SeqNum, OpaqueString]` should also be properly encoded as before and replications should work.
This also fixes a minor annoyance when the _scheduler/{jobs,docs} results returning the default start sequence as `0` (an integer) even though in the majority of case they'd almost always turn into a string. After this we consistently return a string as it would be passed in the `_changes?since=...` parameter.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva merged pull request #4179: Don't double-encode changes sequence strings in the replicator
Posted by GitBox <gi...@apache.org>.
nickva merged PR #4179:
URL: https://github.com/apache/couchdb/pull/4179
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #4179: Don't double-encode changes sequence strings in the replicator
Posted by GitBox <gi...@apache.org>.
nickva commented on PR #4179:
URL: https://github.com/apache/couchdb/pull/4179#issuecomment-1249524421
Log before the PR
```
GET /db1/_changes?feed=normal&style=all_docs&since=%221-g1AAAABzeJzLYWBgYMpgTmHgz8tPSTV0MDQy1zMAQsMckEQiQ1L9____sxIZcCrJYwGSDA1A6j9IZQZzImMuUIDdPDXZ0sAiBVNXFgCBIx1h%22&timeout=10000 200 ok 2
```
With the PR
```
GET /db1/_changes?feed=normal&style=all_docs&since=2-g1AAAACTeJzLYWBgYMpgTmHgz8tPSTV0MDQy1zMAQsMckEQiQ1L9____szKYExlzgQLsJqmWKQaW5pjKcRqRxwIkGRqA1H8Uk8xTky0NLFIwdWUBAEyYJFY&timeout=10000 200 ok 2
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4179: Don't double-encode changes sequence strings in the replicator
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4179:
URL: https://github.com/apache/couchdb/pull/4179#discussion_r973623307
##########
src/couch_replicator/src/couch_replicator_scheduler_job.erl:
##########
@@ -1034,6 +1034,9 @@ get_pending_count_int(#rep_state{source = Db} = St) ->
{ok, Pending} = couch_replicator_api_wrap:get_pending_count(Db, Seq),
Pending.
+seq_format(Seq) ->
Review Comment:
Oh definitely. I'll rename it.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane commented on a diff in pull request #4179: Don't double-encode changes sequence strings in the replicator
Posted by GitBox <gi...@apache.org>.
jaydoane commented on code in PR #4179:
URL: https://github.com/apache/couchdb/pull/4179#discussion_r973613774
##########
src/couch_replicator/src/couch_replicator_api_wrap.erl:
##########
@@ -539,6 +540,7 @@ changes_since(
Options
) ->
Timeout = erlang:max(1000, InactiveTimeout div 3),
+ Seq = couch_replicator_utils:seq_encode(StartSeq),
Review Comment:
Could also use `Encoded` prefix here for clarification?
##########
src/couch_replicator/src/couch_replicator_scheduler_job.erl:
##########
@@ -1034,6 +1034,9 @@ get_pending_count_int(#rep_state{source = Db} = St) ->
{ok, Pending} = couch_replicator_api_wrap:get_pending_count(Db, Seq),
Pending.
+seq_format(Seq) ->
Review Comment:
I'm slightly uneasy about calling it `seq_format` here when it's actually encoded. Why not `seq_encode`?
##########
src/couch_replicator/src/couch_replicator_api_wrap.erl:
##########
@@ -148,7 +148,8 @@ get_pending_count(#httpdb{} = Db, Seq) when is_number(Seq) ->
end
end);
get_pending_count(#httpdb{} = Db, Seq) ->
- Options = [{path, "_changes"}, {qs, [{"since", ?JSON_ENCODE(Seq)}, {"limit", "0"}]}],
+ Seq1 = couch_replicator_utils:seq_encode(Seq),
Review Comment:
Perhaps prefer `EncodedSeq` to clarify how it's different than `Seq`?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4179: Don't double-encode changes sequence strings in the replicator
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4179:
URL: https://github.com/apache/couchdb/pull/4179#discussion_r973623252
##########
src/couch_replicator/src/couch_replicator_api_wrap.erl:
##########
@@ -148,7 +148,8 @@ get_pending_count(#httpdb{} = Db, Seq) when is_number(Seq) ->
end
end);
get_pending_count(#httpdb{} = Db, Seq) ->
- Options = [{path, "_changes"}, {qs, [{"since", ?JSON_ENCODE(Seq)}, {"limit", "0"}]}],
+ Seq1 = couch_replicator_utils:seq_encode(Seq),
Review Comment:
Good idea
##########
src/couch_replicator/src/couch_replicator_api_wrap.erl:
##########
@@ -539,6 +540,7 @@ changes_since(
Options
) ->
Timeout = erlang:max(1000, InactiveTimeout div 3),
+ Seq = couch_replicator_utils:seq_encode(StartSeq),
Review Comment:
Agree, that would be clearer.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4179: Don't double-encode changes sequence strings in the replicator
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4179:
URL: https://github.com/apache/couchdb/pull/4179#discussion_r973623289
##########
src/couch_replicator/src/couch_replicator_api_wrap.erl:
##########
@@ -539,6 +540,7 @@ changes_since(
Options
) ->
Timeout = erlang:max(1000, InactiveTimeout div 3),
+ Seq = couch_replicator_utils:seq_encode(StartSeq),
Review Comment:
Agree, that would be more clear.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org