Skip to content
This repository was archived by the owner on May 4, 2021. It is now read-only.

Bug28897#320

Closed
juga0 wants to merge 10 commits intotorproject:masterfrom
juga0:bug28897
Closed

Bug28897#320
juga0 wants to merge 10 commits intotorproject:masterfrom
juga0:bug28897

Conversation

@juga0
Copy link
Copy Markdown
Contributor

@juga0 juga0 commented Dec 22, 2018

No description provided.

in every measurement.
This removes the need for an extra lock for every measurement
It should also not be depending on a time interval, but on the
number of failures detected.
Not counting number of failures since it would need to modify the
destination or list of at runtime. It should be done in a future
refactor.

Fixes bug #28897. Bugfix v0.3.0
Add methods to store consecutive destination failures and retrieve
the destinations that are still functional.
Since destinations can fail because of Tor circuits, it's not count
individual failures but consecutives one.
Also exit with error if there are no functional destinations left.
The maximum number of consecuitve failures is set to 10, but it
may need to be changed depending on the percentage of circuits and
requests that fail.
Copy link
Copy Markdown
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

This code looks good.

Here are the design changes I think it might need:

  • sbws should be able to recover if the network fails for a short time, then starts working again. Recovery should work if one destination fails, and if all destinations fail.
  • operators might want to configure MAXIMUM_NUMBER_DESTINATION_FAILURES

What do you think?

Comment thread sbws/lib/destination.py Outdated
Comment thread sbws/lib/destination.py
Comment thread sbws/lib/destination.py Outdated
Comment thread sbws/globals.py Outdated
Comment thread sbws/core/scanner.py Outdated
Comment thread sbws/lib/destination.py
Comment thread sbws/core/scanner.py
@juga0
Copy link
Copy Markdown
Contributor Author

juga0 commented Feb 25, 2019

This code looks good.

Here are the design changes I think it might need:

* sbws should be able to recover if the network fails for a short time, then starts working again. Recovery should work if one destination fails, and if all destinations fail.

* operators might want to configure MAXIMUM_NUMBER_DESTINATION_FAILURES

What do you think?

I think that we should open other ticket to implement that in other milestone.
It's true that if there're network problems right now sbws can not recover, but at least it's an easy problem to identify in other ways.

If you agree, then i'd change the other issues.

@teor2345
Copy link
Copy Markdown
Contributor

I think that we should open other ticket to implement that in other milestone.
It's true that if there're network problems right now sbws can not recover, but at least it's an easy problem to identify in other ways.

Ok, can you open a ticket in sbws 1.1 to make destinations recover?

Since destinations will fail forever in 1.0, let's make the default MAXIMUM_NUMBER_DESTINATION_FAILURES equal to 100.

@juga0
Copy link
Copy Markdown
Contributor Author

juga0 commented Feb 26, 2019

I think that we should open other ticket to implement that in other milestone.
It's true that if there're network problems right now sbws can not recover, but at least it's an easy problem to identify in other ways.

Ok, can you open a ticket in sbws 1.1 to make destinations recover?

#29589

Since destinations will fail forever in 1.0, let's make the default MAXIMUM_NUMBER_DESTINATION_FAILURES equal to 100.

ok, done.

@juga0
Copy link
Copy Markdown
Contributor Author

juga0 commented Feb 26, 2019

Can you please do a rebase or merge before the next review, so I can review the code that will be merged to master?

#339

We can close this PR after you review fixups.

@teor2345
Copy link
Copy Markdown
Contributor

The fixups seem fine to me. Closing because #339 is newer.

@teor2345 teor2345 closed this Feb 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants