Ticket31632 043 01 #1593
Ticket31632 043 01 #1593
Conversation
Signed-off-by: David Goulet <dgoulet@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>
If the directory connection dies before being able to upload the descriptor, we'll attempt to retry to upload again to that HSDir. Closes #31632 Signed-off-by: David Goulet <dgoulet@torproject.org>
Pull Request Test Coverage Report for Build 7520
|
| * attempt a re-upload of the corresponding descriptor. This is called when an | ||
| * HS descriptor upload fails. | ||
| * | ||
| * An upload does NOT occure when: |
s/occure/occur/
| } else if (TO_CONN(conn)->purpose == DIR_PURPOSE_UPLOAD_HSDESC && | ||
| conn->hs_ident) { | ||
| log_info(LD_DIR, "Giving up on uploading HS descriptor. Retrying."); | ||
| hs_service_reupload_desc_to_dir(conn->hs_ident, conn->identity_digest); |
Hmm are we talking about infinite retries here? Could this be a problem?
Like, if the network goes down (e.g. mobile), will the service go mental?
Yeah I thought of that as well and figured you would notice it and give me a good idea here lol...
Would be wise to avoid spamming to infinity the reupload to a single HSDir. We would need something like last_hid_serv_request used for clients. And avoid too many retries.
But there is probably an larger problem here which is that the service side does not keep a state of its desc upload. So imagine all 6 upload failed and retried many times but then we stop, chances are that our network is down for instance. When it comes back up, we should notice that we never uploaded and do it again.
Else, the service thinks it made it happen and will wait between 60 and 120 min before retrying...
There is actually an argument now I think to extend this feature to correctly track our HSDir upload service side. What do you think?
Hmm, you are right, I'm not sure what's the ideal behavior here. I'm generally afraid of this code path spinning in an andless loop when the network is down. Perhaps we can add a small delay when the network fails us before we retry. But not sure how hard that would be engineering wise :/
| @@ -759,6 +760,10 @@ connection_dir_client_request_failed(dir_connection_t *conn) | |||
| log_info(LD_DIR, "Giving up on downloading microdescriptors from " | |||
| "directory server at '%s'; will retry", conn->base_.address); | |||
| connection_dir_download_routerdesc_failed(conn); | |||
| } else if (TO_CONN(conn)->purpose == DIR_PURPOSE_UPLOAD_HSDESC && | |||
| conn->hs_ident) { | |||
Has this feature been tested and if yes, how? The lack of unittests is a bit distressing.
Testing that specific feature is sorta hard in a regression test environment (let say Chutney). One would need to close the channel link in the HSDir upload circuit right when the descriptor uploads...
Even with unit test, this is difficult to test. Uploading a descriptor is async (request <-> reply) so at best the unit test would test the correctness of the function based on what we pass it. (which most of the code is already tested except, I give you that, the new hs_service_reupload_desc_to_dir()).
I'll add a unit test that actually confirms that if we end up in the code path of "connection ended badly", we end up re-uploading.
But I won't be able to test that up to the directory upload :S.
Suggestion of real life test: Can you test that by setting up an onion service with a small descriptor upload time, and then shutting down the network before it needs to upload descriptor?
No description provided.
The text was updated successfully, but these errors were encountered: