Accumulo
  1. Accumulo
  2. ACCUMULO-2425

ConcurrentModificationException in master at startup

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.1
    • Fix Version/s: 1.5.2, 1.6.0
    • Component/s: master
    • Labels:
      None

      Description

      I had brought down my accumulo services hard, immediately ran start-here to bring them up again and the master didn't seem to start. Checked the logs and I had

      2014-03-03 16:38:05,336 [master.Master] ERROR: Unexpected exception, exiting
      java.util.ConcurrentModificationException
              at java.util.Hashtable$Enumerator.next(Hashtable.java:1048)
              at org.apache.commons.configuration.AbstractConfiguration.append(AbstractConfiguration.java:1239)
              at org.apache.accumulo.core.conf.Property.getDefaultValue(Property.java:401)
              at org.apache.accumulo.core.conf.DefaultConfiguration.iterator(DefaultConfiguration.java:54)
              at org.apache.accumulo.core.conf.SiteConfiguration.iterator(SiteConfiguration.java:77)
              at org.apache.accumulo.server.conf.ZooConfiguration.iterator(ZooConfiguration.java:127)
              at org.apache.accumulo.server.Accumulo.init(Accumulo.java:125)
              at org.apache.accumulo.server.master.Master.main(Master.java:2303)
              at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
              at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
              at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
              at java.lang.reflect.Method.invoke(Method.java:622)
              at org.apache.accumulo.start.Main$1.run(Main.java:103)
              at java.lang.Thread.run(Thread.java:701)
      

      sitting there, and then it seems the master died. Started it up again and it was fine.

        Issue Links

          Activity

          John Vines created issue -
          Hide
          Eric Newton added a comment - - edited

          These line numbers do not seem to correspond to the 1.5.1 release. In particular, there is no call to AbstractConfiguration.append on line 401 of Property.java. Did you have outstanding modifications?

          Show
          Eric Newton added a comment - - edited These line numbers do not seem to correspond to the 1.5.1 release. In particular, there is no call to AbstractConfiguration.append on line 401 of Property.java. Did you have outstanding modifications?
          Hide
          John Vines added a comment -

          Ah crud, you're right, I had some added Properties I was playing with around encryption. The corrected stack trace around it would be

          2014-03-03 16:38:05,336 [master.Master] ERROR: Unexpected exception, exiting
          java.util.ConcurrentModificationException
                  at java.util.Hashtable$Enumerator.next(Hashtable.java:1048)
                  at org.apache.commons.configuration.AbstractConfiguration.append(AbstractConfiguration.java:1239)
                  at org.apache.accumulo.core.conf.Property.getDefaultValue(Property.java:390)
                  at org.apache.accumulo.core.conf.DefaultConfiguration.iterator(DefaultConfiguration.java:54)
                  at org.apache.accumulo.core.conf.SiteConfiguration.iterator(SiteConfiguration.java:77)
                  at org.apache.accumulo.server.conf.ZooConfiguration.iterator(ZooConfiguration.java:127)
                  at org.apache.accumulo.server.Accumulo.init(Accumulo.java:125)
                  at org.apache.accumulo.server.master.Master.main(Master.java:2303)
                  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                  at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
                  at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                  at java.lang.reflect.Method.invoke(Method.java:622)
                  at org.apache.accumulo.start.Main$1.run(Main.java:103)
                  at java.lang.Thread.run(Thread.java:701)

          Though if you think that added properties would cause this, go ahead and close it as invalid.

          Show
          John Vines added a comment - Ah crud, you're right, I had some added Properties I was playing with around encryption. The corrected stack trace around it would be 2014-03-03 16:38:05,336 [master.Master] ERROR: Unexpected exception, exiting java.util.ConcurrentModificationException at java.util.Hashtable$Enumerator.next(Hashtable.java:1048) at org.apache.commons.configuration.AbstractConfiguration.append(AbstractConfiguration.java:1239) at org.apache.accumulo.core.conf.Property.getDefaultValue(Property.java:390) at org.apache.accumulo.core.conf.DefaultConfiguration.iterator(DefaultConfiguration.java:54) at org.apache.accumulo.core.conf.SiteConfiguration.iterator(SiteConfiguration.java:77) at org.apache.accumulo.server.conf.ZooConfiguration.iterator(ZooConfiguration.java:127) at org.apache.accumulo.server.Accumulo.init(Accumulo.java:125) at org.apache.accumulo.server.master.Master.main(Master.java:2303) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:622) at org.apache.accumulo.start.Main$1.run(Main.java:103) at java.lang. Thread .run( Thread .java:701) Though if you think that added properties would cause this, go ahead and close it as invalid.
          Hide
          John Vines added a comment -

          I have also seen this with the Monitor

          Show
          John Vines added a comment - I have also seen this with the Monitor
          Hide
          John Vines added a comment -

          And a tserver now


          Cheers
          ~John

          Show
          John Vines added a comment - And a tserver now – Cheers ~John
          John Vines made changes -
          Field Original Value New Value
          Assignee John Vines [ vines ]
          John Vines made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          John Vines added a comment -

          Alright, I'm pretty sure the MonitorLog4jWatcher, which calls System.setProperty, is causing this. The CME occurs when there is iteration over the result of System.getProperties, which does no copying for thread safety. I like to think that this should be something handled in Commons configuration (which it's still not in later releases). At the very least, we can address this manually by not relying directly on SystemConfiguration or maybe starting the MonitorLog4jWatcher later

          Show
          John Vines added a comment - Alright, I'm pretty sure the MonitorLog4jWatcher, which calls System.setProperty, is causing this. The CME occurs when there is iteration over the result of System.getProperties, which does no copying for thread safety. I like to think that this should be something handled in Commons configuration (which it's still not in later releases). At the very least, we can address this manually by not relying directly on SystemConfiguration or maybe starting the MonitorLog4jWatcher later
          John Vines made changes -
          Link This issue is broken by ACCUMULO-1440 [ ACCUMULO-1440 ]
          John Vines made changes -
          Link This issue is broken by ACCUMULO-2334 [ ACCUMULO-2334 ]
          Hide
          ASF subversion and git services added a comment -

          Commit f76b8e07ac1f7247eb7a65ad887dce54b9b6aafd in accumulo's branch refs/heads/1.5.2-SNAPSHOT from John Vines
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=f76b8e0 ]

          ACCUMULO-2425 - making SystemConfigurations synchronized until addressed in commons configuration

          Show
          ASF subversion and git services added a comment - Commit f76b8e07ac1f7247eb7a65ad887dce54b9b6aafd in accumulo's branch refs/heads/1.5.2-SNAPSHOT from John Vines [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=f76b8e0 ] ACCUMULO-2425 - making SystemConfigurations synchronized until addressed in commons configuration
          Hide
          ASF subversion and git services added a comment -

          Commit f76b8e07ac1f7247eb7a65ad887dce54b9b6aafd in accumulo's branch refs/heads/1.6.0-SNAPSHOT from John Vines
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=f76b8e0 ]

          ACCUMULO-2425 - making SystemConfigurations synchronized until addressed in commons configuration

          Show
          ASF subversion and git services added a comment - Commit f76b8e07ac1f7247eb7a65ad887dce54b9b6aafd in accumulo's branch refs/heads/1.6.0-SNAPSHOT from John Vines [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=f76b8e0 ] ACCUMULO-2425 - making SystemConfigurations synchronized until addressed in commons configuration
          Hide
          ASF subversion and git services added a comment -

          Commit f76b8e07ac1f7247eb7a65ad887dce54b9b6aafd in accumulo's branch refs/heads/master from John Vines
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=f76b8e0 ]

          ACCUMULO-2425 - making SystemConfigurations synchronized until addressed in commons configuration

          Show
          ASF subversion and git services added a comment - Commit f76b8e07ac1f7247eb7a65ad887dce54b9b6aafd in accumulo's branch refs/heads/master from John Vines [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=f76b8e0 ] ACCUMULO-2425 - making SystemConfigurations synchronized until addressed in commons configuration
          Hide
          John Vines added a comment -

          Took out reliance on SystemConfiguration() and instead opted to use SystemProperties manually with synchronization

          Show
          John Vines added a comment - Took out reliance on SystemConfiguration() and instead opted to use SystemProperties manually with synchronization
          John Vines made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Fix Version/s 1.5.2 [ 12326272 ]
          Fix Version/s 1.6.0 [ 12322468 ]
          Resolution Fixed [ 1 ]
          John Vines made changes -
          Link This issue is related to CONFIGURATION-570 [ CONFIGURATION-570 ]
          Hide
          John Vines added a comment -

          Also, I tested it with a loop on

          timeout 3 bin/accumulo master; if [ "$?" -eq "1" ]; then break; fi;
          Show
          John Vines added a comment - Also, I tested it with a loop on timeout 3 bin/accumulo master; if [ "$?" -eq "1" ]; then break ; fi;
          Hide
          John Vines added a comment -

          I got a response from the commons configuration folks, and http://commons.apache.org/proper/commons-configuration/userguide/howto_basicfeatures.html#Variable_Interpolation was pointed out to me. Changing VFS_CLASSLOADER_CACHE_DIR's default value to have a sys: in front of the two variables allowed me to change getDefaultValue() to

            public String getDefaultValue() {
              if (this.interpolated) {
                PropertiesConfiguration pconf = new PropertiesConfiguration();
                pconf.addProperty("hack_default_value", this.defaultValue);
                String v = pconf.getString("hack_default_value");
                if (this.type == PropertyType.ABSOLUTEPATH)
                  return new File(v).getAbsolutePath();
                else
                  return v;
              } else {
                return getRawDefaultValue();
              }
            }
          

          Theoretically, we could just make it

            public String getDefaultValue() {
                PropertiesConfiguration pconf = new PropertiesConfiguration();
                pconf.addProperty("hack_default_value", this.defaultValue);
                String v = pconf.getString("hack_default_value");
                if (this.type == PropertyType.ABSOLUTEPATH)
                  return new File(v).getAbsolutePath();
                else
                  return v;
            }
          

          and just squelch interpolation entirely. However, I'm hesitant to even make just the first fix because that would actually change the parsing behavior for that one property. We'd need to make it clear that that's how system properties need to be set up.

          Show
          John Vines added a comment - I got a response from the commons configuration folks, and http://commons.apache.org/proper/commons-configuration/userguide/howto_basicfeatures.html#Variable_Interpolation was pointed out to me. Changing VFS_CLASSLOADER_CACHE_DIR's default value to have a sys: in front of the two variables allowed me to change getDefaultValue() to public String getDefaultValue() { if ( this .interpolated) { PropertiesConfiguration pconf = new PropertiesConfiguration(); pconf.addProperty( "hack_default_value" , this .defaultValue); String v = pconf.getString( "hack_default_value" ); if ( this .type == PropertyType.ABSOLUTEPATH) return new File(v).getAbsolutePath(); else return v; } else { return getRawDefaultValue(); } } Theoretically, we could just make it public String getDefaultValue() { PropertiesConfiguration pconf = new PropertiesConfiguration(); pconf.addProperty( "hack_default_value" , this .defaultValue); String v = pconf.getString( "hack_default_value" ); if ( this .type == PropertyType.ABSOLUTEPATH) return new File(v).getAbsolutePath(); else return v; } and just squelch interpolation entirely. However, I'm hesitant to even make just the first fix because that would actually change the parsing behavior for that one property. We'd need to make it clear that that's how system properties need to be set up.
          Hide
          Josh Elser added a comment -

          I like that a lot better.

          Show
          Josh Elser added a comment - I like that a lot better.
          Hide
          John Vines added a comment -

          I'm not sure if trimming it down to my latter example is a concern though. I'm not really worried about users having system variables in their site-xml that they're not using, and the VFS_CLASSLOADER_CACHE_DIR can't be overridden in a similar manner (https://issues.apache.org/jira/browse/ACCUMULO-2449), so I think adding this as a feature is extremely low risk.

          Show
          John Vines added a comment - I'm not sure if trimming it down to my latter example is a concern though. I'm not really worried about users having system variables in their site-xml that they're not using, and the VFS_CLASSLOADER_CACHE_DIR can't be overridden in a similar manner ( https://issues.apache.org/jira/browse/ACCUMULO-2449 ), so I think adding this as a feature is extremely low risk.
          Hide
          John Vines added a comment -

          Actually, further thought- interpolation is ONLY used for defaultValues. What was going on here when that was written? Why do we not want interpolation with user defined settings?

          Show
          John Vines added a comment - Actually, further thought- interpolation is ONLY used for defaultValues. What was going on here when that was written? Why do we not want interpolation with user defined settings?
          Hide
          Christopher Tubbs added a comment -

          What was going on here when that was written?

          Interpolation was added to document the default value only. Originally, VFS_CLASSLOADER_CACHE_DIR just declared System.getProperty("java.io.tmpdir") as part of its value, but that screwed up the auto-generated documentation, by hard-coding the values for the system on which it was built, rather than the values which would actually be used at runtime. This was just a hack to get the documentation correct.

          Why do we not want interpolation with user defined settings?

          I think we do, but that hasn't been added yet. I think that's best done by transitioning the whole configuration to commons-configuration.

          Show
          Christopher Tubbs added a comment - What was going on here when that was written? Interpolation was added to document the default value only. Originally, VFS_CLASSLOADER_CACHE_DIR just declared System.getProperty("java.io.tmpdir") as part of its value, but that screwed up the auto-generated documentation, by hard-coding the values for the system on which it was built, rather than the values which would actually be used at runtime. This was just a hack to get the documentation correct. Why do we not want interpolation with user defined settings? I think we do, but that hasn't been added yet. I think that's best done by transitioning the whole configuration to commons-configuration.
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          2d 20h 49m 1 John Vines 06/Mar/14 18:31
          In Progress In Progress Resolved Resolved
          2h 53m 1 John Vines 06/Mar/14 21:24

            People

            • Assignee:
              John Vines
              Reporter:
              John Vines
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development