Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meta*: Deny requests upon I/O errors #1395

Merged
merged 4 commits into from
Mar 21, 2018
Merged

meta*: Deny requests upon I/O errors #1395

merged 4 commits into from
Mar 21, 2018

Conversation

jfsmig
Copy link
Contributor

@jfsmig jfsmig commented Mar 20, 2018

  • oio-conscience-agent: Consider errors from /stat requests a enough to mark the service down
  • server/transpport_gridd.c: Keep track of IO errors/success timestamps, if the last activity is an error or if the timestamp of the last success is not recent (stalled), consider the volume is faulty.
  • server/transpport_gridd.c: Deny alll but specific local requests with a code 503 that won't fail the general call, and allow the caller to retry on peers.
  • sqlx/sqlx_service.c: Implement a probe in a background thread that periodically checks for directory and file creations.

known limitation: the support of the REQ_PING is really limited in the conscience-agent, the return code is not even checked. We could save the REQ_STAT request if we immediately consider the service is down. then we could even allow the REQ_STAT to return complete information.

@codecov-io
Copy link

codecov-io commented Mar 20, 2018

Codecov Report

Merging #1395 into 4.1.x will decrease coverage by 0.15%.
The diff coverage is 76%.

Impacted file tree graph

@@            Coverage Diff             @@
##            4.1.x    #1395      +/-   ##
==========================================
- Coverage   76.72%   76.57%   -0.16%     
==========================================
  Files         298      298              
  Lines       55601    55651      +50     
  Branches     6770     6776       +6     
==========================================
- Hits        42658    42612      -46     
+ Misses      12689    12685       -4     
- Partials      254      354     +100
Impacted Files Coverage Δ
sqliterepo/repository.c 72.5% <ø> (-0.25%) ⬇️
server/transport_gridd.h 100% <ø> (ø) ⬆️
server/network_server.c 81.24% <100%> (+0.04%) ⬆️
server/transport_gridd.c 78.17% <75%> (-0.02%) ⬇️
sqlx/sqlx_service.c 82.4% <75%> (-0.52%) ⬇️
proxy/cache_actions.c 60% <0%> (-9.15%) ⬇️
proxy/sqlx_actions.c 56.64% <0%> (-6.94%) ⬇️
proxy/m2_actions.c 82.92% <0%> (-4.47%) ⬇️
oio/account/client.py 77.77% <0%> (-4.45%) ⬇️
proxy/dir_actions.c 73.33% <0%> (-3.5%) ⬇️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 442494d...1f43ce7. Read the comment docs.

@@ -71,8 +71,6 @@ def get_stats(self):
result = self._parse_func(resp.read())
else:
raise Exception("status code != 200: %s" % resp.status)
except Exception as e:
self.logger.debug("get_stats error: %s", e)
Copy link
Member

Choose a reason for hiding this comment

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

The return statement in the finally block will prevent any exception from being raised from this method. It should be moved here.

goto label_io_error;
}
if (err)
g_clear_error(&err);
Copy link
Member

Choose a reason for hiding this comment

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

err won't be set by g_file_set_contents if it returns true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants