Details

    • Type: Task Task
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.6, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      just some minor improvements:

      • always use EOFException when its eof
      • always include the inputstream too so we know filename etc
      • use FileNotFoundException consistently in CFS when a sub-file is not found
      1. LUCENE-3680_more.patch
        4 kB
        Robert Muir
      2. LUCENE-3680.patch
        12 kB
        Robert Muir

        Activity

        Hide
        Robert Muir added a comment -

        I think this patch is safe, I'll bring it up to speed and commit it tomorrow.

        Show
        Robert Muir added a comment - I think this patch is safe, I'll bring it up to speed and commit it tomorrow.
        Hide
        Uwe Schindler added a comment -

        Cool +1

        We should better check all Closeables...

        Show
        Uwe Schindler added a comment - Cool +1 We should better check all Closeables...
        Hide
        Robert Muir added a comment -

        patch with more consistency fixes.

        I put TODO's in the lockfactory exceptions where a directory is a regular file (not sure if we should be using NoSuchDirectoryException here?)

        Show
        Robert Muir added a comment - patch with more consistency fixes. I put TODO's in the lockfactory exceptions where a directory is a regular file (not sure if we should be using NoSuchDirectoryException here?)
        Hide
        Uwe Schindler added a comment -

        I agree, Closeable.close() is allowed to be closed multiple times, while additional calls must have no effect. So the first throws is wrong, should be simple return.

        Show
        Uwe Schindler added a comment - I agree, Closeable.close() is allowed to be closed multiple times, while additional calls must have no effect. So the first throws is wrong, should be simple return.
        Hide
        Robert Muir added a comment -

        I committed the patch (backporting), but i found a few more problems relating to
        inconsistent use of exceptions when accessing a closed resource.

        here are some from 3.x's compoundfiledirectory:

          public synchronized void close() throws IOException {
            if (stream == null)
              throw new IOException("Already closed");
          ...
          public synchronized IndexInput openInput(String id, int readBufferSize) throws IOException {
            if (stream == null)
              throw new IOException("Stream closed");
        

        I think the close() one is wrong since it impls closeable, but in the other case we should use AlreadyClosedException (like other directories).
        Ill look around for more of these and create another patch.

        Show
        Robert Muir added a comment - I committed the patch (backporting), but i found a few more problems relating to inconsistent use of exceptions when accessing a closed resource. here are some from 3.x's compoundfiledirectory: public synchronized void close() throws IOException { if (stream == null) throw new IOException("Already closed"); ... public synchronized IndexInput openInput(String id, int readBufferSize) throws IOException { if (stream == null) throw new IOException("Stream closed"); I think the close() one is wrong since it impls closeable, but in the other case we should use AlreadyClosedException (like other directories). Ill look around for more of these and create another patch.
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        Uwe Schindler added a comment -

        +1 Thanks for taking care!

        Show
        Uwe Schindler added a comment - +1 Thanks for taking care!

          People

          • Assignee:
            Robert Muir
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development