Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9536

OldBackupDirectory timestamp init bug causes NPEs from SnapShooter?

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.3, 7.0
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None

      Description

      On IRC, a 6.2.0 user reported getting an NPE from SnapShooter.deleteOldBackups L244, with the only other frame of the stacktrace being lambda$createSnapAsync$1 L196 (it was a screenshot, not text easily cut/paste here)

      The problematic L244 is...

        if (obd.getTimestamp().isPresent()) {
      

      ..and i believe the root of the issue is that while getTimestamp() is declared to return an Optional<Date>, there is no guarantee that the Optional instance is itself non-null...

         private Optional<Date> timestamp;
      
        public OldBackupDirectory(URI basePath, String dirName) {
          this.dirName = Preconditions.checkNotNull(dirName);
          this.basePath = Preconditions.checkNotNull(basePath);
          Matcher m = dirNamePattern.matcher(dirName);
          if (m.find()) {
            try {
              this.timestamp = Optional.of(new SimpleDateFormat(SnapShooter.DATE_FMT, Locale.ROOT).parse(m.group(1)));
            } catch (ParseException e) {
              this.timestamp = Optional.empty();
            }
          }
        }
      

      Allthough i'm not 100% certain, I believe the way the user was triggering this bug was by configuring classic replication configured with something like <str name="replicateAfter">commit</str> – so that usage may be neccessary to trigger the exception?

      Alternatively: perhaps this exception gets logged the first time anyone tries to use any code that involves SnapShooter – and after that a timestamp file is created and teh problem neer manifests itself again?

        Issue Links

          Activity

          Hide
          hossman Hoss Man added a comment -

          pretty sure the code in SOLR-7374 introduced this NPE

          Show
          hossman Hoss Man added a comment - pretty sure the code in SOLR-7374 introduced this NPE
          Hide
          hossman Hoss Man added a comment -

          Varun: can you take a look at this and sanity check my analysis here?

          Show
          hossman Hoss Man added a comment - Varun: can you take a look at this and sanity check my analysis here?
          Hide
          hgadre Hrishikesh Gadre added a comment - - edited

          Hoss Man Yes this is correct. We need to initialize the timestamp field during the construction. I have prepared a patch and running the unit tests currently. Will submit the patch in next couple of hours.

          Show
          hgadre Hrishikesh Gadre added a comment - - edited Hoss Man Yes this is correct. We need to initialize the timestamp field during the construction. I have prepared a patch and running the unit tests currently. Will submit the patch in next couple of hours.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user hgadre opened a pull request:

          https://github.com/apache/lucene-solr/pull/81

          SOLR-9536 Initialize timestamp field with Optional.empty() to avoid an NPE

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/hgadre/lucene-solr SOLR-9536_fix

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/lucene-solr/pull/81.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #81


          commit b3f52a7d4dd4823c2ba2e54ae75dc0f50533dcf8
          Author: Hrishikesh Gadre <hgadre@cloudera.com>
          Date: 2016-09-20T02:58:21Z

          SOLR-9536 Initialize timestamp field with Optional.empty() to avoid an NPE


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user hgadre opened a pull request: https://github.com/apache/lucene-solr/pull/81 SOLR-9536 Initialize timestamp field with Optional.empty() to avoid an NPE You can merge this pull request into a Git repository by running: $ git pull https://github.com/hgadre/lucene-solr SOLR-9536 _fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/81.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #81 commit b3f52a7d4dd4823c2ba2e54ae75dc0f50533dcf8 Author: Hrishikesh Gadre <hgadre@cloudera.com> Date: 2016-09-20T02:58:21Z SOLR-9536 Initialize timestamp field with Optional.empty() to avoid an NPE
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit e152575f5ea5ea798ca989c852afb763189dee60 in lucene-solr's branch refs/heads/master from markrmiller
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e152575 ]

          SOLR-9536: OldBackupDirectory timestamp field needs to be initialized to avoid NPE.

          Show
          jira-bot ASF subversion and git services added a comment - Commit e152575f5ea5ea798ca989c852afb763189dee60 in lucene-solr's branch refs/heads/master from markrmiller [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e152575 ] SOLR-9536 : OldBackupDirectory timestamp field needs to be initialized to avoid NPE.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 22117ddde47bc5b646ec1f0e732b51479a8e8bab in lucene-solr's branch refs/heads/branch_6x from markrmiller
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=22117dd ]

          SOLR-9536: OldBackupDirectory timestamp field needs to be initialized to avoid NPE.

          This closes #81.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 22117ddde47bc5b646ec1f0e732b51479a8e8bab in lucene-solr's branch refs/heads/branch_6x from markrmiller [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=22117dd ] SOLR-9536 : OldBackupDirectory timestamp field needs to be initialized to avoid NPE. This closes #81.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c15c8af66db5c2c84cdf95520a61f78d512c5911 in lucene-solr's branch refs/heads/master from markrmiller
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c15c8af ]

          SOLR-9536: Add hossman to CHANGES.

          Show
          jira-bot ASF subversion and git services added a comment - Commit c15c8af66db5c2c84cdf95520a61f78d512c5911 in lucene-solr's branch refs/heads/master from markrmiller [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c15c8af ] SOLR-9536 : Add hossman to CHANGES.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c1d1e6098a6c4dcc2d6f031b1299545f79972794 in lucene-solr's branch refs/heads/branch_6x from markrmiller
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c1d1e60 ]

          SOLR-9536: Add hossman to CHANGES.

          Show
          jira-bot ASF subversion and git services added a comment - Commit c1d1e6098a6c4dcc2d6f031b1299545f79972794 in lucene-solr's branch refs/heads/branch_6x from markrmiller [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c1d1e60 ] SOLR-9536 : Add hossman to CHANGES.
          Hide
          markrmiller@gmail.com Mark Miller added a comment -

          Thanks guys!

          Show
          markrmiller@gmail.com Mark Miller added a comment - Thanks guys!
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Closing after 6.3.0 release.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Closing after 6.3.0 release.

            People

            • Assignee:
              markrmiller@gmail.com Mark Miller
              Reporter:
              hossman Hoss Man
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development