Skip to content
Snippets Groups Projects
  1. Nov 16, 2018
  2. Nov 15, 2018
  3. Nov 16, 2018
  4. Nov 15, 2018
    • Matt Harbison's avatar
      lfs: make the exception messages consistent · 93e5d18251d6
      Matt Harbison authored
      I don't love that it repeats 'HTTP Error' in an already long message, but I
      doubt that we should assume that it will always say that on the original
      exception message.
      93e5d18251d6
    • Matt Harbison's avatar
      lfs: handle URLErrors to add additional information · 380f5131ee7b
      Matt Harbison authored
      Sometimes the blob server is hit first (e.g. on push), and sometimes it's hit
      last (e.g. pull).  Throw in depth first subrepo operations, and things quickly
      get insane.  It wasn't even mentioning LFS, so just saying "connection refused"
      can be confusing- especially if the blob server is a secondary server and
      connecting to the repo server works.
      
      The exception handler for the transfer handler will print the full path to the
      blob, but that seems fine given that it might be necessary to debug a second
      server.  (We don't yet support a standalone blob server, so the handler for the
      Batch API will cover 99.9% of the current problems.  But it might as well be
      handled now while I'm thinking about it.)
      
      The function for translating to a message was mostly borrowed from
      scmutil.catchall().
      380f5131ee7b
    • Matt Harbison's avatar
      lfs: improve the hints for common errors in the Batch API · 9f78d10742af
      Matt Harbison authored
      The previous message was too debug-ish and less action oriented than a hint
      should be.  The remaining errors that aren't handled are more along the lines of
      programming errors (not using POST, bad accept type, etc), so I'm not bothering
      with that.
      
      The friendly errors purposely use `self.baseurl` instead of the full Batch API
      endpoint because I'd expect some copy/paste/modify on the part of the user here,
      and it would be more confusing if '/objects/batch' magically appeared, but
      shouldn't be used in the config setting.  It still seems like the right thing
      for debugging in the catchall case.
      9f78d10742af
    • Matt Harbison's avatar
      lfs: provide more Batch API error info via a hint in the raised exception · 8863f08c1630
      Matt Harbison authored
      A coworker had a typo in `lfs.url`, forgot it was even set because usually the
      blob server is inferred, and then got a 404.  It would have been easier to debug
      with the failing URL printed.
      8863f08c1630
    • Matt Harbison's avatar
      scmutil: display the optional hint when handling StorageError in catchall() · 4ec8bee15930
      Matt Harbison authored
      Other than CensoredNodeError (which is also a StorageError), it looks like all
      exceptions with a hint display them.  I'm not sure that it makes sense to have a
      hint for censored nodes, so I'm not bothering with that.
      
      It looks like nobody is using this yet, as the tests don't change.
      4ec8bee15930
    • Boris Feld's avatar
      sparse-revlog: align endrevidx usages in the _slicechunktosize · 2eb48aa0acce
      Boris Feld authored
      All "startrevidx..endrevidx" ranges in this function are now half-open.
      2eb48aa0acce
    • Boris Feld's avatar
      sparse-revlog: use `span` variable as intended · fd1d41ccbe38
      Boris Feld authored
      The variable was planned to be used in the while condition but was not used
      yet.
      fd1d41ccbe38
    • Matt Harbison's avatar
      tests: stabilize test-commandserver.t on Windows · 6d0fdeda43f7
      Matt Harbison authored
      It looks like new test coverage in 054d0fcba2c4, rather than a code change.
      6d0fdeda43f7
    • Matt Harbison's avatar
      histedit: conditionalize the imports of 'fcntl' and 'termios' · 291080871f50
      Matt Harbison authored
      The recent import of chistedit in c36175456350 made Windows sad.  I'm not sure
      if there's other stuff that needs to be done here (e.g. change the default
      interface), but this makes the tests run again.
      
      It would have been nicer if the error message indicated these modules were the
      problem, but instead it said "*** failed to import extension histedit: No module
      named histedit".  I'm not sure if there's anything we can do about that.
      291080871f50
  5. Nov 16, 2018
    • Boris Feld's avatar
      logtoprocess: update commandfinish options arguments · 1243295fcc3a
      Boris Feld authored
      d2c997b8001f changed the logtoprocess API with the effect of not exposing the
      positional arguments to the logtoprocess scripts anymore.
      
      We have some scripts that use the duration and return code of the
      "commandfinish" event to monitor hg calls. Update the logging of the
      "commandfinish" to expose those values as options argument, which will be
      accessible as `OPT_RETURN_CODE` and `OPT_DURATION` in logtoprocess arguments.
      
      The code has been formatted with Black.
      
      Differential Revision: https://phab.mercurial-scm.org/D5282
      1243295fcc3a
  6. Nov 15, 2018
  7. Nov 11, 2018
  8. Nov 09, 2018
    • Boris Feld's avatar
      sparse-revlog: rework the way we enforce chunk size limit · 9c3c697267db
      Boris Feld authored
      We move from a O(N) algorithm to a O(log(N)) algorithm.
      
      The previous algorithm was traversing the whole delta chain, looking for the
      exact point where it became too big. This would result in most of the delta
      chain to be traversed.
      
      Instead, we now use a "binary" approach, slicing the chain in two until we
      have a chunk of the appropriate size.
      
      We still keep the previous algorithm for the snapshots part. There are few of
      them and they are large bits of data distant from each other. So the previous
      algorithm should work well in that case.
      
      To take a practical example of restoring manifest revision '59547c40bc4c' for
      a reference NetBeans repository (using sparse-revlog). The media time of the
      step `slice-sparse-chain` of `perfrevlogrevision` improve from 1.109 ms to
      0.660 ms.
      9c3c697267db
  9. Nov 13, 2018
  10. Nov 15, 2018
  11. Nov 05, 2018
  12. Nov 14, 2018
  13. Nov 11, 2018
  14. Nov 14, 2018
    • Augie Fackler's avatar
      tests: fix bytes/str issue I introduced when adding this test · ab59cc71f80c
      Augie Fackler authored
      # skip-blame just b prefixes for py3
      
      Differential Revision: https://phab.mercurial-scm.org/D5271
      ab59cc71f80c
    • Kyle Lippincott's avatar
      shelve: use matcher to restrict prefetch to just the modified files · 29e4a77b5305
      Kyle Lippincott authored
      Shelve currently operates by:
      - make a temp commit
      - identify all the bases necessary to shelve, put them in the bundle
      - use exportfile to export the temp commit to the bundle ('file' here means
        "export to this fd", not "export this file")
      - remove the temp commit
      
      exportfile calls prefetchfiles, and prefetchfiles uses a matcher to restrict
      what files it's going to prefetch; if it's not provided, it's alwaysmatcher.
      This means that `hg shelve` in a remotefilelog repo can possibly download the
      file contents of everything in the repository, even when it doesn't need to. It
      luckily is restricted to the narrowspec (if there is one), but this is still a
      lot of downloading that's just unnecessary, especially if there's a "smart"
      VCS-aware filesystem involved.
      
      exportfile is called with exactly one revision to emit, so we're just
      restricting it to prefetching the files from that revision. The base revisions
      having separate files should not be a concern since they're handled already;
      example:
      
      commit 10 is draft and modifies foo/a.txt and foo/b.txt
      commit 11 is draft and modifies foo/a.txt
      my working directory that I'm shelving modifies foo/b.txt
      
      By the time we get to exportfile, commit 10 and 11 are already handled, so the
      matcher only specifying foo/b.txt does not cause any problems. I verified this
      by doing an `hg unbundle` on the bundle that shelve produces, and getting the
      full contents of those commits back out, instead of just the files that were
      modified in the shelve.
      
      Differential Revision: https://phab.mercurial-scm.org/D5268
      29e4a77b5305
  15. Nov 13, 2018
    • Gregory Szorc's avatar
      revlog: automatically read from opened file handles · e9293c5f8bb9
      Gregory Szorc authored
      The revlog reading code commonly opens a new file handle for
      reading on demand. There is support for passing a file handle
      to revlog.revision(). But it is marked as an internal argument.
      
      When revlogs are written, we write() data as it is available. But
      we don't flush() data until all revisions are written.
      
      Putting these two traits together, it is possible for an in-process
      revlog reader during active writes to trigger the opening of a new
      file handle on a file with unflushed writes. The reader won't have
      access to all "available" revlog data (as it hasn't been flushed).
      And with the introduction of the previous patch, this can lead to
      the revlog raising an error due to a partial read.
      
      I witnessed this behavior when applying changegroup data (via
      `hg pull`) before issue6006 was fixed via different means. Having
      this and the previous patch in play would have helped cause errors
      earlier rather than manifesting as hash verification failures.
      
      While this has been a long-standing issue, I believe the relatively
      new delta computation code has tickled it into being more common.
      This is because the new delta computation code will compute deltas
      in more scenarios. This can lead to revlog reading. While the delta
      computation code is probably supposed to reuse file handles, it
      appears it isn't doing so in all circumstances.
      
      But the issue runs deeper than that. Theoretically, any code can
      access revision data during revlog writes. It appears we were just
      getting lucky that it wasn't. (The "add revision callback" passed to
      addgroup() provides an avenue to do this.)
      
      If I changed the revlog's behavior to not cache the full revision
      text or to clear caches after revision insertion during addgroup(),
      I was able to produce crashes 100% of the time when writing changelog
      revisions. This is because changelog's add revision callback attempts
      to resolve the revision data to access the changed files list. And
      without the revision's fulltext being cached, we performed a revlog
      read, which required opening a new file handle. This attempted to read
      unflushed data, leading to a partial read and a crash.
      
      This commit teaches the revlog to store the file handles used for
      writing multiple revisions during addgroup(). It also teaches the
      code for resolving a file handle when reading to use these handles,
      if available. This ensures that *any* reads (regardless of their
      source) use the active writing file handles, if available. These
      file handles have access to the unflushed data because they wrote it.
      This allows reads to complete without issue.
      
      Differential Revision: https://phab.mercurial-scm.org/D5267
      e9293c5f8bb9
    • Gregory Szorc's avatar
      revlog: detect incomplete revlog reads · 87a872555e90
      Gregory Szorc authored
      _readsegment() is supposed to return N bytes of revlog revision
      data starting at a file offset. Surprisingly, its behavior before
      this patch never verified that it actually read and returned N
      bytes! Instead, it would perform the read(), then return whatever
      data was available. And even more surprisingly, nothing in the
      call chain appears to have been validating that it received all
      the data it was expecting.
      
      This behavior could lead to partial or incomplete revision chunks
      being operated on. This could result in e.g. cached deltas being
      applied against incomplete base revisions. The delta application
      process would happily perform this operation. Only hash
      verification would detect the corruption and save us.
      
      This commit changes the behavior of raw revlog reading to validate
      that we actually read() the number of bytes that were requested.
      We will raise a more specific error faster, rather than possibly
      have it go undetected or manifest later in the call stack, at
      delta application or hash verification.
      
      Differential Revision: https://phab.mercurial-scm.org/D5266
      87a872555e90
  16. Oct 30, 2018
    • Gregory Szorc's avatar
      revlog: use single file handle when de-inlining revlog · 39369475445c
      Gregory Szorc authored
      _getsegmentforrevs() will eventually call into _datareadfp() to
      resolve a file handle to read revision data. If no file handle
      is passed into _getsegmentforrevs(), it opens a new one.
      
      Explicit is better than implicit.
      
      This commit changes _enforceinlinesize() to open a file handle
      explicitly when converting inline revlogs to split revlogs and
      to pass this file handle into _getsegmentforrevs().
      
      I haven't measured, but this change should improve performance,
      as we no longer reopen the revlog for reading for every revision
      in the revlog when it is converted from inline to split. Instead,
      we open it at most once and use it for the duration of the
      operation. That being said, I /think/ the chunk cache may mitigate
      the number of file opens required.
      
      Differential Revision: https://phab.mercurial-scm.org/D5265
      39369475445c
  17. Nov 13, 2018
Loading