Return error if trying to open secondary on missing or inaccessible primary #8200
Conversation
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
riversand963
force-pushed
the
secondary-switch-manifest
branch
from
2285fbc
to
dbd3a73
Compare
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
riversand963
force-pushed
the
secondary-switch-manifest
branch
from
dbd3a73
to
06b45d8
Compare
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
riversand963
force-pushed
the
secondary-switch-manifest
branch
from
06b45d8
to
54c6df5
Compare
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| s = Status::TryAgain( | ||
| "The primary may have switched to a new MANIFEST and deleted the old " | ||
| "one."); |
There was a problem hiding this comment.
Should we retry it, instead of reporting to user? Maybe the secondary should take a snapshot of manifest/CURRENT file at the beginning.
There was a problem hiding this comment.
If the secondary checks for the presence of MANIFEST via FileExists() and then tries to open it, there is always a possibility that the primary deletes the MANIFEST in between, thus the secondary will always likely hit a missing MANIFEST file. Similarly, after checking the presence of MANIFEST but before taking a hard-link, it is possible that the MANIFEST is deleted. We should always handle this situation.
Retry is possible, but I think we should have an interface for application to set retry policy. Since we do not have the interface yet, and it is beyond the scope of this PR. Future change of adding retry logic will be a behavior/contract change for TryCatchUpWithPrimary().
riversand963
force-pushed
the
secondary-switch-manifest
branch
from
54c6df5
to
919e2a2
Compare
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@riversand963 merged this pull request in 2e53881. |
If the primary's CURRENT file is missing or inaccessible, the secondary should not hang
trying repeatedly to switch to the next MANIFEST.
Test plan:
make check