Summary:
Certain IO errors are surely data corruption - the data can be read
(no permission or resource errors), but does not meet expectations
(format or range).
This fixes some cases covered by an upcoming test about `repair`.
Reviewed By: xavierd
Differential Revision: D17742001
fbshipit-source-id: 8436d477a25efe09bd5a763371df913acbfccc68
Summary: This should fix lints about `stat` being redefined.
Reviewed By: kulshrax
Differential Revision: D17766500
fbshipit-source-id: ca326ae9d0c922c60ac26f0e367a70e11d20fcd3
Summary: The old code incorrectly only respects the last argument of `setconfig`.
Reviewed By: xavierd
Differential Revision: D17766375
fbshipit-source-id: ec70c10eb62803c9f3d8dde5d88f91d076a55049
Summary:
https://www.python.org/dev/peps/pep-0479/
If you are in a generator (a function that uses "yield") you are never allowed to raise StopIteration instead you should just `return` or pass out of scope (implicit return None)
Reviewed By: thatch
Differential Revision: D17749640
fbshipit-source-id: 9f1be673cf877ff193a0379a0208d037dd2d7bae
Summary:
Rust manifests are the future. Tests work well with them and we are looking
ahead at rolling them out. To protect against regressions on the Rust manifest
side we need to have an automated test story.
I think that the most effective thing we can do is to enable Rust Manifest for
all tests. The C++ codebase is not seeing any kind of modification and we plan
to delete as soon as Rust manifests roll out.
Reviewed By: xavierd
Differential Revision: D17728414
fbshipit-source-id: 59979d02e3cece83e73569a43d6fdbb7a29dc66d
Summary: This is a better way to wrap smartlog functionality.
Reviewed By: mitrandir77
Differential Revision: D17686738
fbshipit-source-id: 770b32c19c89a59aee0f8b25251faaae7685a4dd
Summary: The graphnode templatekw must be wrapped in the keywords dict, and the phabstatus template needs to work if the requested revision is hidden.
Reviewed By: mitrandir77
Differential Revision: D17685575
fbshipit-source-id: 5e4a8b45dfe6564af3f72de3511d834aa498d154
Summary:
Right now, not being able to find the mmap file can be seen as data corruption.
The only case that NotFound needs special handling is at open time.
This fixes some cases covered by an upcoming test about `repair`.
Reviewed By: xavierd
Differential Revision: D17741999
fbshipit-source-id: 1bd7c65c5a6381892723b31e2e749b22081e96d2
Summary: Now all `indexedlog` APIs use the new new Error type.
Reviewed By: xavierd
Differential Revision: D17732136
fbshipit-source-id: 8d306a08d8e8052d1c5e68fc5f05a9eed5c7d21f
Summary: This provides more details, and makes callsites simpler.
Reviewed By: xavierd
Differential Revision: D17732127
fbshipit-source-id: 0fe6dedee4ebb8874ea95505c86d8b107e3367ff
Summary:
Similar to the previous change, add context for Log APIs.
This shows error context that might replace backtrace. For example, run:
cargo test --test low_fileno_limit -- --nocapture
An example error looks like:
"/tmp/.tmpjrsfQt/rotatelog/1/index-key1": cannot duplicate file descriptor
in ChecksumTable::try_clone
in Index::try_clone
Index.path = "/tmp/.tmpjrsfQt/rotatelog/1/index-key1"
in Log::sync
Log.dir = Some("/tmp/.tmpjrsfQt/rotatelog/1")
Caused by 1 errors:
- Os { code: 24, kind: Other, message: "Too many open files" }
(Ignoring whitespace will make this diff much easier to review)
Reviewed By: xavierd
Differential Revision: D17732124
fbshipit-source-id: b0d500652d80b4a4755453c69bc05d467ecbdf90
Summary:
Since we lost backtrace by opting out failure, it'd be nice to restore some
"backtrace" information like what Index function is being called.
This diff adds it. It also includes more context like what key is being looked
up so it might actually be more useful than backtrace.
(Ignoring whitespace will make this diff much easier to review)
Reviewed By: xavierd
Differential Revision: D17732126
fbshipit-source-id: 8e5a2c714bee8a943076818f0cff3a21498a954e
Summary: This basically involves adding contexts for io::Error and other error types.
Reviewed By: xavierd
Differential Revision: D17732130
fbshipit-source-id: 79fb3b93d57562f1922f3990a8bda0018d2675e8
Summary: The new utlity function makes it easier to deal with mmap errors.
Reviewed By: xavierd
Differential Revision: D17732139
fbshipit-source-id: 93c8209b983d51198ebb367db983a2e9bc498d63
Summary: This makes it easier to lock a directory and makes error handling easier.
Reviewed By: xavierd
Differential Revision: D17732133
fbshipit-source-id: a404d41c0aaee7aad43271433f1352a8aa06bccb
Summary:
Migrate the remaining part of Index functions to use the new Error type. This
gives us an accurate view about whether an error indicates data corruption or
not, and makes the code more friendly - it works with `std::error::Error` now.
Reviewed By: xavierd
Differential Revision: D17705168
fbshipit-source-id: 8ae518602e7379d121e718a08127f0873f2e2423
Summary:
Migrate some return types from Fallible to the new Result. The main changes are
the way `io::Result` gets handled. The new API enforces attaching a `path` and
a message to them.
Reviewed By: xavierd
Differential Revision: D17705163
fbshipit-source-id: d060bdb2846a75c588b99201fd07ca3872f3a358
Summary:
Migrate more free-form errors handling like `data_error`, `parameter_error`
to the new Error type.
Reviewed By: xavierd
Differential Revision: D17705164
fbshipit-source-id: 45560a96e36fb5e83a9e365506e27c201f9448a6
Summary:
Migrate `range_error` and `verify_checksum` to the `IndexBuf` trait so they
all get path information on error. Remove the free-form `range_error` and
`verify_checksum` functions.
Reviewed By: xavierd
Differential Revision: D17705165
fbshipit-source-id: 556fda8081c69b6beccc8c666902810a90635231
Summary:
A lot of functions take (buf, checksum) tuple, instead of `Index` for input.
That is to avoid issues where borrowing the entire `Index` forbids modifying
other fields in `Index`.
However, not taking `Index` means it cannot figure out the file path on error.
To solve both problems, this diff defines a trait that is a subset of Index
including (on-disk buf, checksum, path). Then migrate functions from using
(buf, checksum) to the new trait (if it only needs to read from the on-disk
buffer), or &Index (if it also needs to work with in-memory dirty/mutable
data).
Reviewed By: xavierd
Differential Revision: D17705166
fbshipit-source-id: 90bde88142ea3718a2093beb02b8030d725a0e15
Summary:
Change some `range_error` to `Index::range_error`.
The new error is better because it includes path information.
Reviewed By: xavierd
Differential Revision: D17705162
fbshipit-source-id: 1de1c7cdd730fcf7c6c39e9e5840939fa561bc33
Summary:
Change `read_bitmap_unchecked` and `read_raw_int_unchecked` to use the new
Error type. Change their function signature from taking `&[u8]` to taking
`&Index` so we can get the file path in the error message.
Reviewed By: xavierd
Differential Revision: D17705167
fbshipit-source-id: 82bcbe21061cdf993d5c7f9867941c1f936166e5
Summary:
Migrate to the new Error type so we can know whether an error is considered
as a data corruption. The new Error should also provide more explicit error
messages.
(This diff is easier to review if whitespace changes are all ignored)
Reviewed By: xavierd
Differential Revision: D17696536
fbshipit-source-id: bfceffbf75a75940a90c914da7914a601d75a747
Summary:
`io::Result` is widely used in indexedlog internal and they need to be
converted to `Result`.
This diff defines the conversion function. It enforces 2 context parameters:
- File path.
- What operations is it? This is needed since we will lose the backtrace.
Reviewed By: xavierd
Differential Revision: D17696533
fbshipit-source-id: d9417a6b65cbfbb5d6d7d1c6449ddd13e3035b5c
Summary:
I need to make RotateLog understand whether errors occured in Log/std::io/Index
are data corruption or not. To be explicit, I defined a `is_data_corruption`
method. Downcasting a chain does not look like a confident solution (ex. less
confident to check that it covers all possible cases).
There are other motivations for this change:
- `failure`: it is unfriendly in a low level library; it requires callsites to
use failure, too. `failure` is less maintained - it still provides the nice
backtrace feature but it's more friendly if libraries just use std Error (we
lose backtrace inside the library, but hopefully the errors are in a high
quality so backtrace in the application is enough for debugging).
- Error with multi-sources. Both std and failure Error provides one slot for
"cause". Sometimes it's desirable to use multiple slots. For example,
RotateLog::open fails to read existing logs, and also fails to auto recover
by creating a new log. In that case, ideally we keep both errors in the
returned type.
Reviewed By: xavierd
Differential Revision: D17696532
fbshipit-source-id: 0387b3a3b71f097b1a3dc2dcc7671a43c465abb2
Summary:
This test checks that the RotateLog can still be opened, and read if the fileno
limit crashes other writer processes "randomly".
Reviewed By: xavierd
Differential Revision: D17676318
fbshipit-source-id: e08528189adfa260047c357c723c87735592ec8f
Summary: This logs more contexts for errors that might help debugging.
Reviewed By: xavierd
Differential Revision: D17670723
fbshipit-source-id: d22fb53689c0766b99aa344659a15148017212ad
Summary:
On Windows, a simple `hg log -l1 -p --pager=off` shows that 10% of the type
is spent querying the fstype. It turns out that the number of paths that the
function is being called with is very small, and thus is a perfect candidate
for using lrucachefunc.
Reviewed By: quark-zju
Differential Revision: D17715347
fbshipit-source-id: 6b3b790eb1821d464f8c9d6761f26af5be749761
Summary: These 4 checks are done by black, no need to have duplicated code.
Reviewed By: quark-zju
Differential Revision: D17714260
fbshipit-source-id: 9cb4aa97e97216435e68eb9ed726a26631f04875
Summary:
The function is now just wrapping a PyErr into a PythonError, let's inline this
bit into the code.
Reviewed By: quark-zju
Differential Revision: D17729379
fbshipit-source-id: ad569cee03497fab710f760d0fa09e0ea61fe208
Summary: This is no longer needed.
Reviewed By: quark-zju
Differential Revision: D17729378
fbshipit-source-id: 43d24df01dfae0449473d33fa851114e951197b0
Summary:
Instead of using KeyError to indicate that data isn't found, let's use an
Option. The Option type better encode that data is missing without having to do
a potentially error prone downcast, this may also enable us to set
RUST_BACKTRACE=1 everywhere as we won't except errors to happen often anymore,
previously, Mercurial will slow to a crawl due to the many KeyError being
thrown around.
I initially wanted to keep the change small to help reviews, but that didn't
really work out, as the dependencies on the `DataStore`/`HistoryStore` traits
are all over the place...
Reviewed By: quark-zju
Differential Revision: D17728486
fbshipit-source-id: de89c4fc441fd12ff37cc248e2230e4a1403ce44
Summary:
This pessimizes the subprocess creation (see https://bugs.python.org/issue35757) when performing
sockname discovery and isn't strictly required: watchman will
perform an equivalent operation for itself IFF it needs to spawn
the daemon.
Reviewed By: quark-zju
Differential Revision: D17728332
fbshipit-source-id: 5e00c82969aeaee38d93c44aba4959c6733cce25
Summary:
My previous change, D17083282, broke addremove on certain case
insensitive filesystems. This was caused by me naively checking for file
existence, which didn't take into account case differences. Let's normalize the
file first and compare it. This appears to be what the old dirstate.walk code
does, though it's a huge mess so it's hard to be certain.
Reviewed By: singhsrb
Differential Revision: D17731308
fbshipit-source-id: 430edc1663748bff8db491bfc8ed609e2f340c60
Summary:
This error is not particularly useful for users to see, and eventually there
will be cases where it is in fact expected that some paths will be invalid
(this is particularly useful on hosts where the same config is read by
different processes with a different environment and / or mount namespace).
Reviewed By: HarveyHunt
Differential Revision: D17710604
fbshipit-source-id: 588c3653df91a84035f044af9455c6ac83305090
Summary:
This is a short-term fix to help surface the real errors.
Instead of silently deleting or renaming data, surface the error so we
can get crash/traceback logged, and can login to investigate the broken
state.
Reviewed By: xavierd
Differential Revision: D17729511
fbshipit-source-id: b066ef12101aa742b4834bfd2e90bcb42fa15aff
Summary: Now that the `File` type is part of the crate's public API, it should be placed in the `files` module along with all the other exported file-related types (such as `FileMetadata`).
Reviewed By: xavierd
Differential Revision: D17726709
fbshipit-source-id: 4e3c0100ca765a7145f9eea49aa0b7ff11496c4b
Summary:
test-check complains about the test using `raise Exception.`
For the purposes of the test, `assert` seems more appropriate.
Reviewed By: DurhamG
Differential Revision: D17724080
fbshipit-source-id: 2831a384c1e91012ee58bced47f026cd74995e86
Summary: When a downloaded manifest node fails to validate, investigating the issue generally requires the p1/p2 nodes (to manually compute the hash and compare it to the expected value). As such, let's print these out as part of the error message.
Reviewed By: xavierd
Differential Revision: D17724746
fbshipit-source-id: 0b1eb8d5344c0376a5895745dcdfb1092ad06321
Summary:
Essentially, snapshots are being synced exactly like heads are.
A dedicated table in CC DB etc.
Reviewed By: markbt
Differential Revision: D17345491
fbshipit-source-id: a1e65c2a4815f437a11da68f9d46c27f046453a9
Summary:
This is the last use of dirstate.walk, aside from the actual status
function. This is necessary for an upcoming diff which deletes dirstate.walk
entirely.
Reviewed By: quark-zju
Differential Revision: D17170796
fbshipit-source-id: 9a81631d12f5f174b43a10e083bbbda2bd377854
Summary:
When inserting a glob into match._tree, it marks the tree as
'unsurerecursive' if the glob contains a complex component (i.e. a component
that contains "/"). If the glob was only one component, that check was not
executed against the component and therefore we could end up in a situation
where we didn't mark the tree as unsure. An example would be `'{*,{b,m}*/*}k'`,
which returned False for tree.visitdir("beans/black") when it should've returned
True.
Reviewed By: quark-zju
Differential Revision: D17118623
fbshipit-source-id: 50b6adbbecda13461c13c06a6e7d8523f8b29a2d
Summary:
patternmatcher can be used for non-exact and non-prefix patterns, but
the current visitdir implementation will return False unless a prefix pattern or
exact patterns are provided. A future change makes dirstate.status more reliant
on correct matcher behavior, and this breaks tests.
The fix is to return True (meaning that the directory should be visited)
whenever the pattern is not an explicit list of files. Hopefully we can move
this to our rust matcher in the future which will be able to optimize this case
around globs and regular expressions.
Reviewed By: quark-zju
Differential Revision: D17116369
fbshipit-source-id: d65b08060143efc3f4bbde3198b0c505a8ff4ec7
Summary:
In a future diff we'll be restricting the dirstate interface. We'd like
to drop dirstate.walk, so let's remove it's use from `hg addremove`.
Reviewed By: quark-zju
Differential Revision: D17083282
fbshipit-source-id: 010483b24e1c23f8f8086bbc5f931095d94080de
