nickva commented on a change in pull request #6:
URL: https://github.com/apache/couchdb-ibrowse/pull/6#discussion_r638210107
##########
File path: src/ibrowse_http_client.erl
##########
@@ -465,7 +465,7 @@ accumulate_response(Data, #state{reply_buffer = RepBuf,
make_tmp_filename(true) ->
DownloadDir = ibrowse:get_config_value(download_dir,
filename:absname("./")),
- {A,B,C} = now(),
+ {A,B,C} = {erlang:unique_integer([positive, monotonic]), 0, 0},
Review comment:
Good call. Here it also expects it to be strictly monotonic. But seeing
that this creates a file made think of another case when the erlang VM is
restarted. In that case the counter will be reset to 0 and so filename might
collide. I think we might want to keep at least some connection to wall clock
time here.
Perhaps something like:
```
strictly_monotonic_timestamp() ->
{A, B, C} = erlang:timestamp(),
Unique = erlang:unique_integer([positive, monotonic]),
{A, B, C + Unique}.
```
##########
File path: src/ibrowse_http_client.erl
##########
@@ -1870,7 +1870,7 @@ cancel_timer(Ref, {eat_message, Msg}) ->
end.
make_req_id() ->
- now().
+ {erlang:unique_integer([positive, monotonic]), 0, 0}.
Review comment:
I saw at least once request ID is sent in a
[header](https://github.com/apache/couchdb-ibrowse/blob/3e0e5fffdb63f79d7a55ea38d384fdf4fbede6cf/src/ibrowse_http_client.erl#L970)
and could potentially be logged. And since unique integer counter is reset to
0 on every VM restart the behavior might change. So I think we could use the
same scheme as for the filename (pull it into a helper function) and could
re-use the same logic.
This might still introduce a minor incompatibility if users ever
introspected the value of `ReqId` and expected it to be a proper timestamp. It
doesn't seem like ibrowse documents `ReqId` to be a `timestamp()` and the type
spec in `browse.erl` just says it's a generic
[term()](https://github.com/apache/couchdb-ibrowse/blob/master/src/ibrowse.erl#L162).
But if you feel like could add it as a comment somewhere in the code or just
in the commit message.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
