Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-alpha4
    • Component/s: None
    • Target Version/s:
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Hide
      CodecRegistry uses ServiceLoader to dynamically load all implementations of RawErasureCoderFactory. In Hadoop 3.0, there are several built-in implementations, and user can also provide self-defined implementations with the corresponding resource files.
      For each codec, user can configure the order of the implementations with the configuration keys:
      `io.erasurecode.codec.rs.rawcoders` for the default RS codec,
      `io.erasurecode.codec.rs-legacy.rawcoders` for the legacy RS codec,
      `io.erasurecode.codec.xor.rawcoders` for the XOR codec.
      User can also configure self-defined codec with the configuration key like:
      `io.erasurecode.codec.self-defined.rawcoders`.
      For each codec, Hadoop will use the implementation according to the order configured. If the former implementation fails, it will fall back to call the latter one. The order is defined by a list of coder names separated by commas. The names for the built-in implementations are:
      `rs_native` and `rs_java` for the default RS codec, of which the former is a native implementation which leverages Intel ISA-L library, which is the default implementation and the latter is the implementation in pure Java,
      `rs-legacy_java` for the legacy RS codec, which is the default implementation in pure Java,
      `xor_native` and `xor_java` for the XOR codec, of which the former is the Intel ISA-L implementation which is the default one and the latter in pure Java.
      Show
      CodecRegistry uses ServiceLoader to dynamically load all implementations of RawErasureCoderFactory. In Hadoop 3.0, there are several built-in implementations, and user can also provide self-defined implementations with the corresponding resource files. For each codec, user can configure the order of the implementations with the configuration keys: `io.erasurecode.codec.rs.rawcoders` for the default RS codec, `io.erasurecode.codec.rs-legacy.rawcoders` for the legacy RS codec, `io.erasurecode.codec.xor.rawcoders` for the XOR codec. User can also configure self-defined codec with the configuration key like: `io.erasurecode.codec.self-defined.rawcoders`. For each codec, Hadoop will use the implementation according to the order configured. If the former implementation fails, it will fall back to call the latter one. The order is defined by a list of coder names separated by commas. The names for the built-in implementations are: `rs_native` and `rs_java` for the default RS codec, of which the former is a native implementation which leverages Intel ISA-L library, which is the default implementation and the latter is the implementation in pure Java, `rs-legacy_java` for the legacy RS codec, which is the default implementation in pure Java, `xor_native` and `xor_java` for the XOR codec, of which the former is the Intel ISA-L implementation which is the default one and the latter in pure Java.

      Description

      This is a follow-on task for HADOOP-13010 as discussed over there. There may be some better approach allowing to customize and configure erasure coders than the current having raw coder factory, as Colin P. McCabe suggested. Will copy the relevant comments here to continue the discussion.

      1. HADOOP-13200.02.patch
        34 kB
        Tim Yao
      2. HADOOP-13200.03.patch
        33 kB
        Tim Yao
      3. HADOOP-13200.04.patch
        37 kB
        Tim Yao
      4. HADOOP-13200.05.patch
        38 kB
        Tim Yao
      5. HADOOP-13200.06.patch
        40 kB
        Tim Yao
      6. HADOOP-13200.07.patch
        42 kB
        Tim Yao
      7. HADOOP-13200.08.patch
        41 kB
        Tim Yao
      8. HADOOP-13200.09.patch
        41 kB
        Tim Yao
      9. HADOOP-13200.10.patch
        41 kB
        Tim Yao
      10. HADOOP-13200.11.patch
        45 kB
        Tim Yao

        Issue Links

          Activity

          Hide
          drankye Kai Zheng added a comment -

          Copied from here by Kai Zheng:

          Hi Colin,

          Thanks for the comments. About the factories, I have to clarify the real problem in details and hope this works since the f2f discussion isn't going into details due to time constraint.

          We may have the following codecs in the 1st level:
          rs-legacy, rs-default (both belonging to RS)
          xor,
          hh or hitchhiker,
          lrc,
          ...

          And for each codec, it may use one or more raw coders, but each of such coders may use different implementations. For example, for the rs-default codec, we have two coder implementations (the pure java one and the isa-l one). Users may add their own coder implementation for a codec, maybe for better performance.

          So that's why I would have a configuration key like this:
          o.a.h.io.erasurecode.codec.(codec-name).rawcoder: (whatever value to be used to create or load the coder).

          Currently we configured the factory to create the encoder and decoder for a coder implementation, I agree there could be better option here, and while discussing about this in details with Andrew yesterday in the SF office, wonder if we could achieve the effect avoding the factories using java service loader.

          First, we can add codec-name and coder-name to the raw coder, so each coder will have a codec-name and coder-name when it's created.

          Then we have the built-in coders of fixed codec-name and coder-name. Customized coders will be loaded via service loader.

          Eventually we will have all the raw erasure coders loaded and created, then we can setup a mapping between codec-name and coder-name, coder-name and the coder-class or instance.

          Does this sound good to you? If it works, then we might do this in a follow-on task?

          Thanks again!

          Show
          drankye Kai Zheng added a comment - Copied from here by Kai Zheng : Hi Colin, Thanks for the comments. About the factories, I have to clarify the real problem in details and hope this works since the f2f discussion isn't going into details due to time constraint. We may have the following codecs in the 1st level: rs-legacy, rs-default (both belonging to RS) xor, hh or hitchhiker, lrc, ... And for each codec, it may use one or more raw coders, but each of such coders may use different implementations. For example, for the rs-default codec, we have two coder implementations (the pure java one and the isa-l one). Users may add their own coder implementation for a codec, maybe for better performance. So that's why I would have a configuration key like this: o.a.h.io.erasurecode.codec.(codec-name).rawcoder: (whatever value to be used to create or load the coder). Currently we configured the factory to create the encoder and decoder for a coder implementation, I agree there could be better option here, and while discussing about this in details with Andrew yesterday in the SF office, wonder if we could achieve the effect avoding the factories using java service loader. First, we can add codec-name and coder-name to the raw coder, so each coder will have a codec-name and coder-name when it's created. Then we have the built-in coders of fixed codec-name and coder-name. Customized coders will be loaded via service loader. Eventually we will have all the raw erasure coders loaded and created, then we can setup a mapping between codec-name and coder-name, coder-name and the coder-class or instance. Does this sound good to you? If it works, then we might do this in a follow-on task? Thanks again!
          Hide
          drankye Kai Zheng added a comment -

          Copied partly here by Colin P. McCabe:

          I think these are really configuration questions, not questions about how the code should be structured. What does the user actually need to configure? If the user just configures a coder implementation, does that fully determine the codec which is being used? If so, we should have only one configuration knob-- coder. If a coder could be used for multiple codecs, then we need to have at least two knobs that the user can configure-- one for codec, and another for coder. Once we know what the configuration knobs are, we probably only need one or two functions to create the objects we need based on a Configuration object, not a whole mess of factory objects.

          Show
          drankye Kai Zheng added a comment - Copied partly here by Colin P. McCabe : I think these are really configuration questions, not questions about how the code should be structured. What does the user actually need to configure? If the user just configures a coder implementation, does that fully determine the codec which is being used? If so, we should have only one configuration knob-- coder. If a coder could be used for multiple codecs, then we need to have at least two knobs that the user can configure-- one for codec, and another for coder. Once we know what the configuration knobs are, we probably only need one or two functions to create the objects we need based on a Configuration object, not a whole mess of factory objects.
          Hide
          drankye Kai Zheng added a comment -

          Hi Colin P. McCabe and Andrew Wang,

          Thanks a lot for your time and the nice off-line talk! It's great to meet you face to face personally.

          As the refactoring work of HADOOP-13010 incurs significant change, the work (the ISA-L coder) in HADOOP-11540 needs to be changed much accordingly. I'm currently work on it. As we discussed, it would be great to include the work in HADOOP-13010 and HADOOP-11540 in the first cut of Hadoop 3 so I will try to speed up to meet with the general schedule. Meanwhile let's continue the discussion about how to support the customization and configuration of erasure coders, and guess the work could be included in a subsequent Hadoop 3 release like alpha2? Kindly help clarify if anything mistaken. Thanks!

          Show
          drankye Kai Zheng added a comment - Hi Colin P. McCabe and Andrew Wang , Thanks a lot for your time and the nice off-line talk! It's great to meet you face to face personally. As the refactoring work of HADOOP-13010 incurs significant change, the work (the ISA-L coder) in HADOOP-11540 needs to be changed much accordingly. I'm currently work on it. As we discussed, it would be great to include the work in HADOOP-13010 and HADOOP-11540 in the first cut of Hadoop 3 so I will try to speed up to meet with the general schedule. Meanwhile let's continue the discussion about how to support the customization and configuration of erasure coders, and guess the work could be included in a subsequent Hadoop 3 release like alpha2? Kindly help clarify if anything mistaken. Thanks!
          Hide
          lewuathe Kai Sasaki added a comment -

          Hi Kai Zheng.
          Is there any update on refactoring side? Although HADOOP-11540 seems to be fixed soon, you have several JIRAs in HADOOP-11842. If there are some tasks which can be distributed, I can help. It might be the time to restart the discussion of configuration design. Thanks!

          Show
          lewuathe Kai Sasaki added a comment - Hi Kai Zheng . Is there any update on refactoring side? Although HADOOP-11540 seems to be fixed soon, you have several JIRAs in HADOOP-11842 . If there are some tasks which can be distributed, I can help. It might be the time to restart the discussion of configuration design. Thanks!
          Hide
          drankye Kai Zheng added a comment -

          Hey Kai Sasaki,

          Thanks for the ping. Currently my side is mainly focusing on some performance specific tasks to suite the native coder. I will be back to the refactoring like after that. Regarding this, I haven't the perfect idea yet. Your thoughts are welcome!

          Show
          drankye Kai Zheng added a comment - Hey Kai Sasaki , Thanks for the ping. Currently my side is mainly focusing on some performance specific tasks to suite the native coder. I will be back to the refactoring like after that. Regarding this, I haven't the perfect idea yet. Your thoughts are welcome!
          Hide
          drankye Kai Zheng added a comment -

          Kai,

          It would be great if you could share some workload. Would you want to work on HADOOP-13061 doing the refactoring for erasure coders? If yes, I can transfer it to you. Thanks.

          Show
          drankye Kai Zheng added a comment - Kai, It would be great if you could share some workload. Would you want to work on HADOOP-13061 doing the refactoring for erasure coders? If yes, I can transfer it to you. Thanks.
          Hide
          lewuathe Kai Sasaki added a comment -

          Kai Zheng
          Sure, I can work on HADOOP-13061 and make a initial patch based on above discussion.
          Though customizability and configuration is the main topic of this JIRA, HADOOP-13061 refactoring needs to also consider that point, I think. I will list up the main refactoring point in HADOOP-13061. Please give me some feedback if necessary. Thanks!

          Show
          lewuathe Kai Sasaki added a comment - Kai Zheng Sure, I can work on HADOOP-13061 and make a initial patch based on above discussion. Though customizability and configuration is the main topic of this JIRA, HADOOP-13061 refactoring needs to also consider that point, I think. I will list up the main refactoring point in HADOOP-13061 . Please give me some feedback if necessary. Thanks!
          Hide
          drankye Kai Zheng added a comment -

          Great, Kai! Let's discuss about it in that issue. I will assign it to you.

          Show
          drankye Kai Zheng added a comment - Great, Kai! Let's discuss about it in that issue. I will assign it to you.
          Hide
          andrew.wang Andrew Wang added a comment -

          Found this while doing a JIRA triage. I think the related JIRAs have been completed, is it time to revisit this one?

          Show
          andrew.wang Andrew Wang added a comment - Found this while doing a JIRA triage. I think the related JIRAs have been completed, is it time to revisit this one?
          Hide
          andrew.wang Andrew Wang added a comment -

          Marking ec must do's as blockers for alpha3

          Show
          andrew.wang Andrew Wang added a comment - Marking ec must do's as blockers for alpha3
          Hide
          andrew.wang Andrew Wang added a comment -

          Hi, anything we can do help push this one forwards? Looks like it's been inactive for a while now.

          Show
          andrew.wang Andrew Wang added a comment - Hi, anything we can do help push this one forwards? Looks like it's been inactive for a while now.
          Hide
          drankye Kai Zheng added a comment -

          Roughly the problem is, how to configure a raw coder impl for a codec. A raw coder impl includes both an encoder and decoder. Currently it uses a raw coder factory to combine an encoder and decoder together to represent a raw coder impl for a codec. Available codecs are rs-default, rs-legacy, xor, hh-xor and etc. raw coder impls for rs-default codec are: RSRawErasureCoderFactory and NativeRSRawErasureCoderFactory. More impls for a codec could be provided/added in future. The issue originated from the discussion with Colin and he disliked the current way using the factory method. It was meant to figure out a way to get rid of the raw coder factories.

          I don’t have a perfect solution for this in mind yet. Some related ideas so far:
          1. Dynamically combine the encoder/decoder name given codec name and other info, roughly suggested by Colin but I may not catch him quite exactly. It doesn’t look to me very attractive because there is no easy or intuitive way to reduce a raw coder name directly from some configuration properties. Given the raw coder name is reduced and then the corresponding encoder/decoder class name can be out so we can create the needed encoder/decoder instances directly.

          2. Combine encoder and decoder together, suggested by ATM somewhere. If we combine encoder and decoder together, then we can directly save or avoid the factory. It sounds good for some raw coder impls but not for others. Some raw encoder/decoder impl is pretty complex, if we combine them the resultant class will be pretty large and hard to maintain. Generally, decoding logic will be much complex than encoding. An extreme example, LRC codec, the both encoding and decoding logic will be quite complicated, so better to be separate.

          Note in current approach, to configure something for a codec or a raw coder impl for the codec, the configuration property starts with something like io.erasurecode.codec.rs.rawcoder.*

          Andrew Wang, what's your thought? Thanks!

          Show
          drankye Kai Zheng added a comment - Roughly the problem is, how to configure a raw coder impl for a codec. A raw coder impl includes both an encoder and decoder. Currently it uses a raw coder factory to combine an encoder and decoder together to represent a raw coder impl for a codec. Available codecs are rs-default, rs-legacy, xor, hh-xor and etc. raw coder impls for rs-default codec are: RSRawErasureCoderFactory and NativeRSRawErasureCoderFactory. More impls for a codec could be provided/added in future. The issue originated from the discussion with Colin and he disliked the current way using the factory method. It was meant to figure out a way to get rid of the raw coder factories. I don’t have a perfect solution for this in mind yet. Some related ideas so far: 1. Dynamically combine the encoder/decoder name given codec name and other info, roughly suggested by Colin but I may not catch him quite exactly. It doesn’t look to me very attractive because there is no easy or intuitive way to reduce a raw coder name directly from some configuration properties. Given the raw coder name is reduced and then the corresponding encoder/decoder class name can be out so we can create the needed encoder/decoder instances directly. 2. Combine encoder and decoder together, suggested by ATM somewhere. If we combine encoder and decoder together, then we can directly save or avoid the factory. It sounds good for some raw coder impls but not for others. Some raw encoder/decoder impl is pretty complex, if we combine them the resultant class will be pretty large and hard to maintain. Generally, decoding logic will be much complex than encoding. An extreme example, LRC codec, the both encoding and decoding logic will be quite complicated, so better to be separate. Note in current approach, to configure something for a codec or a raw coder impl for the codec, the configuration property starts with something like io.erasurecode.codec.rs.rawcoder.* Andrew Wang , what's your thought? Thanks!
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks for the writeup Kai, this is a tricky API issue as you note, and there are multiple opinions on the matter. This is my first time looking at it, so some perhaps basic questions:

          IIUC, our raw interfaces are like the following:

          RawErasureEncoder:

          • encode
          • getters for ECOptions
          • some configs (direct buffers, change inputs, verbose dump)
          • release

          RawErasureDecoder

          • decode
          • getters for ECOptions
          • some configs (direct buffers, change inputs, verbose dump)
          • release

          There's some overlap here, so if we provided both encode and decode in a single RawErasureCoder interface, it might be less code and simple to configure. This would be like the ErasureCoder interface that shares functionality between the higher-level encoder and decoder. I saw in a comment that the raw coders are stateless, so it seems okay from an API perspective.

          Overall though I think the current system is okay. The factory is the single entry point to configuring a RawCoder. Right now the RawEncoders and RawDecoders live in separate Java files, but implementers are allowed to make them nested classes of the RawErasureCoderFactory.

          Maybe this is what Colin wanted, since the factory classes look trivial by themselves. I experimented by putting the NativeRS raw encoder and raw decoder into their Factory class, and it looks okay since they're pretty small. I also made the nested classes private, to ensure they're created via the factory and paired correctly.

          What do you think?

          Other questions/comments from looking at this code:

          • We also should rename RSRawEncoderLegacy to RSLegacyRawEncoder and RSRawErasureCoderFactoryLegacy to RSLegacyErasureCoderFactory, to match the naming of other subclasses.
          • TestRawEncoderBase, should this use a configured factory to get the raw coders, rather than referencing the raw coders directly? I didn't check other usages, but it seems like we should be creating via the appropriate factory whenever possible.
          Show
          andrew.wang Andrew Wang added a comment - Thanks for the writeup Kai, this is a tricky API issue as you note, and there are multiple opinions on the matter. This is my first time looking at it, so some perhaps basic questions: IIUC, our raw interfaces are like the following: RawErasureEncoder: encode getters for ECOptions some configs (direct buffers, change inputs, verbose dump) release RawErasureDecoder decode getters for ECOptions some configs (direct buffers, change inputs, verbose dump) release There's some overlap here, so if we provided both encode and decode in a single RawErasureCoder interface, it might be less code and simple to configure. This would be like the ErasureCoder interface that shares functionality between the higher-level encoder and decoder. I saw in a comment that the raw coders are stateless, so it seems okay from an API perspective. Overall though I think the current system is okay. The factory is the single entry point to configuring a RawCoder. Right now the RawEncoders and RawDecoders live in separate Java files, but implementers are allowed to make them nested classes of the RawErasureCoderFactory. Maybe this is what Colin wanted, since the factory classes look trivial by themselves. I experimented by putting the NativeRS raw encoder and raw decoder into their Factory class, and it looks okay since they're pretty small. I also made the nested classes private, to ensure they're created via the factory and paired correctly. What do you think? Other questions/comments from looking at this code: We also should rename RSRawEncoderLegacy to RSLegacyRawEncoder and RSRawErasureCoderFactoryLegacy to RSLegacyErasureCoderFactory, to match the naming of other subclasses. TestRawEncoderBase, should this use a configured factory to get the raw coders, rather than referencing the raw coders directly? I didn't check other usages, but it seems like we should be creating via the appropriate factory whenever possible.
          Hide
          drankye Kai Zheng added a comment -

          Thanks Andrew for the nice thoughts.

          I saw in a comment that the raw coders are stateless ...

          Sorry I think this should be clarified a little bit and it's partly true: 1) When I said it's stateless, I meant given a coder instance, the encode/decode call is stateless, that is to say, for a certain group of data to encode/decode, we can have concurrent threads to call encode/decode so to speed up. 2) The coder instance has states, specific to schema, configuration (and erasure parameters for decoder).

          The biggest concern to merge encoder and decoder together is, the states can be mixed and incur some runtime overhead. For example, both encoder and decoder can maintain big coding matrix in core cache; when merged, the memory consumption can be double (yes it can be avoided by different code path but then complicated). In HDFS side, a coder instance just serves one role, either encode or decode, not both the same time.

          Maybe this is what Colin wanted, since the factory classes look trivial by themselves.

          I guess you're right, that's most likely what Colin thought. The factory classes are trivial right now, but I think it can evolve to make more sense: 1) In HADOOP-13665 I think we can do the fallback thing here elegantly, where encoder/decoder creating can fail and it can try next one configured in a list; 2) Quite some time ago when I played with micor benchmarking of the coders, I found cache coder instance can help in performance, and it's good to do it here than elsewhere like in CodecUtil.

          I experimented by putting the NativeRS raw encoder and raw decoder into their Factory class, and it looks okay since they're pretty small.

          It's a very interesting try. Yes the native RS encoder/decoder are small, for other coders they may not. I thought coders and coder factories may evolve to be bigger in future, for coders if we want to support incremental encoding/decoding, then more codes will be added. As HH codec indicates, if any party supports complex codec, the encode/decode logic can be much complex.

          We also should rename RSRawEncoderLegacy to RSLegacyRawEncoder ...

          Right, agree.

          it seems like we should be creating via the appropriate factory whenever possible.

          Can't agree any more.

          Overall though I think the current system is okay. The factory is the single entry point to configuring a RawCoder.

          I'm glad the current system works for you. Do you think it's good to fix above problems for this issue?

          Thanks!

          Show
          drankye Kai Zheng added a comment - Thanks Andrew for the nice thoughts. I saw in a comment that the raw coders are stateless ... Sorry I think this should be clarified a little bit and it's partly true: 1) When I said it's stateless, I meant given a coder instance, the encode/decode call is stateless, that is to say, for a certain group of data to encode/decode, we can have concurrent threads to call encode/decode so to speed up. 2) The coder instance has states, specific to schema, configuration (and erasure parameters for decoder). The biggest concern to merge encoder and decoder together is, the states can be mixed and incur some runtime overhead. For example, both encoder and decoder can maintain big coding matrix in core cache; when merged, the memory consumption can be double (yes it can be avoided by different code path but then complicated). In HDFS side, a coder instance just serves one role, either encode or decode, not both the same time. Maybe this is what Colin wanted, since the factory classes look trivial by themselves. I guess you're right, that's most likely what Colin thought. The factory classes are trivial right now, but I think it can evolve to make more sense: 1) In HADOOP-13665 I think we can do the fallback thing here elegantly, where encoder/decoder creating can fail and it can try next one configured in a list; 2) Quite some time ago when I played with micor benchmarking of the coders, I found cache coder instance can help in performance, and it's good to do it here than elsewhere like in CodecUtil . I experimented by putting the NativeRS raw encoder and raw decoder into their Factory class, and it looks okay since they're pretty small. It's a very interesting try. Yes the native RS encoder/decoder are small, for other coders they may not. I thought coders and coder factories may evolve to be bigger in future, for coders if we want to support incremental encoding/decoding, then more codes will be added. As HH codec indicates, if any party supports complex codec, the encode/decode logic can be much complex. We also should rename RSRawEncoderLegacy to RSLegacyRawEncoder ... Right, agree. it seems like we should be creating via the appropriate factory whenever possible. Can't agree any more. Overall though I think the current system is okay. The factory is the single entry point to configuring a RawCoder. I'm glad the current system works for you. Do you think it's good to fix above problems for this issue? Thanks!
          Hide
          andrew.wang Andrew Wang added a comment -

          Hi Kai,

          Would be great to file JIRAs to handle the separate small issues noticed. I also spent some more time thinking about this.

          High-level problem: determine what raw encoder and decoder to use for a coder. Our current system:

          coder --------> rawcoder factory method ---------> factory ---------> raw encoder / decoder
                 rawcoder                         reflection          hardcode
                 factory
                 class name
          

          If we replace the reflection step with a registry, we can save the per-rawcoder factory classes:

          coder ----------> rawcoder factory registry --------> factory ----------> raw encoder + decoder
                 rawcoder                              lookup            hardcode
                 factory
                 name
          
          • Raw coder factories would be identified by an additional getName() interface.
          • The registry is a singleton that maps coders to a map of rawcoder factories, keyed by getName()
          • Registry is prepopulated with the built-in factories; these can be private nested classes of the registry, or held in a new class.
          • The list of pluggable raw coder factory classes are specified in a config key. We classload these at startup and trigger their static initializers, which register them with the registry. We could enforce namespacing of pluggable raw coder names to future-proof.

          Since nothing in the registry is config-dependent, I think it's a safe singleton. Config-specific logic is handled outside the Registry or in static methods.

          I think this might also help with implementing caching later, since it's centralized and avoids reflection after initialization.

          Thoughts?

          Show
          andrew.wang Andrew Wang added a comment - Hi Kai, Would be great to file JIRAs to handle the separate small issues noticed. I also spent some more time thinking about this. High-level problem: determine what raw encoder and decoder to use for a coder. Our current system: coder --------> rawcoder factory method ---------> factory ---------> raw encoder / decoder rawcoder reflection hardcode factory class name If we replace the reflection step with a registry, we can save the per-rawcoder factory classes: coder ----------> rawcoder factory registry --------> factory ----------> raw encoder + decoder rawcoder lookup hardcode factory name Raw coder factories would be identified by an additional getName() interface. The registry is a singleton that maps coders to a map of rawcoder factories, keyed by getName() Registry is prepopulated with the built-in factories; these can be private nested classes of the registry, or held in a new class. The list of pluggable raw coder factory classes are specified in a config key. We classload these at startup and trigger their static initializers, which register them with the registry. We could enforce namespacing of pluggable raw coder names to future-proof. Since nothing in the registry is config-dependent, I think it's a safe singleton. Config-specific logic is handled outside the Registry or in static methods. I think this might also help with implementing caching later, since it's centralized and avoids reflection after initialization. Thoughts?
          Hide
          drankye Kai Zheng added a comment -

          HADOOP-14261 was opened for the smaller issues.

          Show
          drankye Kai Zheng added a comment - HADOOP-14261 was opened for the smaller issues.
          Hide
          drankye Kai Zheng added a comment -

          Hi Andrew,

          Thanks for your thoughts! I guess I roughly catch your points, so try to align with yours:
          1. How about for the raw coder factory registry we use ServiceLoader pattern? Say each raw coder factory will be picked up from class loaders by the loader. All the loaded raw coder factories will be populated into the singleton registry.

          2. Assume each raw coder factory contributes to a codec, and the codec name can be determined by RawCoderFactory->getCodecName(), so the mapping between codec and its implemented raw coder factories can be determined while initialization phase.

          3. Considering a codec can have multiple raw coder factories and users may want to specify the priority list, how about each raw coder or its factory is assigned a unique name? The name can be determined by RawCoderFactory->getName(). The name can be used in the priority list.

          Show
          drankye Kai Zheng added a comment - Hi Andrew, Thanks for your thoughts! I guess I roughly catch your points, so try to align with yours: 1. How about for the raw coder factory registry we use ServiceLoader pattern? Say each raw coder factory will be picked up from class loaders by the loader. All the loaded raw coder factories will be populated into the singleton registry. 2. Assume each raw coder factory contributes to a codec, and the codec name can be determined by RawCoderFactory->getCodecName() , so the mapping between codec and its implemented raw coder factories can be determined while initialization phase. 3. Considering a codec can have multiple raw coder factories and users may want to specify the priority list, how about each raw coder or its factory is assigned a unique name? The name can be determined by RawCoderFactory->getName() . The name can be used in the priority list.
          Hide
          andrew.wang Andrew Wang added a comment -

          Your suggestion is even better! I didn't find ServiceLoader when I searched before, and it looks like the perfect way of populating the registry.

          Looking forward to the patch

          Show
          andrew.wang Andrew Wang added a comment - Your suggestion is even better! I didn't find ServiceLoader when I searched before, and it looks like the perfect way of populating the registry. Looking forward to the patch
          Hide
          drankye Kai Zheng added a comment -

          Thanks Andrew for the confirm and assigned to Tim Yao for the patch per offline discussion.

          Show
          drankye Kai Zheng added a comment - Thanks Andrew for the confirm and assigned to Tim Yao for the patch per offline discussion.
          Hide
          timmyyao Tim Yao added a comment -

          Add CoderRegistry to realize a dynamic class loader of RawErasureCoderFactory using ServiceLoader.

          Show
          timmyyao Tim Yao added a comment - Add CoderRegistry to realize a dynamic class loader of RawErasureCoderFactory using ServiceLoader.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 8s HADOOP-13200 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12862795/HADOOP-13200.01.patch
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12080/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 8s HADOOP-13200 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12862795/HADOOP-13200.01.patch Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12080/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          drankye Kai Zheng added a comment -

          Hi Tim,

          Thanks for your quick work on this! The patch looks not in good form. You need also a rebase.

          Show
          drankye Kai Zheng added a comment - Hi Tim, Thanks for your quick work on this! The patch looks not in good form. You need also a rebase.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 12s HADOOP-13200 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863760/HADOOP-13200.02.patch
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12114/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 12s HADOOP-13200 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863760/HADOOP-13200.02.patch Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12114/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 13m 17s trunk passed
          +1 compile 16m 54s trunk passed
          +1 checkstyle 0m 37s trunk passed
          +1 mvnsite 1m 12s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 1m 40s trunk passed
          +1 javadoc 0m 51s trunk passed
          +1 mvninstall 0m 47s the patch passed
          +1 compile 15m 48s the patch passed
          +1 javac 15m 48s the patch passed
          -0 checkstyle 0m 35s hadoop-common-project/hadoop-common: The patch generated 30 new + 2 unchanged - 0 fixed = 32 total (was 2)
          +1 mvnsite 1m 2s the patch passed
          +1 mvneclipse 0m 20s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 1m 35s the patch passed
          +1 javadoc 0m 49s the patch passed
          -1 unit 8m 9s hadoop-common in the patch failed.
          +1 asflicense 0m 34s The patch does not generate ASF License warnings.
          66m 37s



          Reason Tests
          Failed junit tests hadoop.io.erasurecode.coder.TestRSErasureCoder
            hadoop.ha.TestZKFailoverController
            hadoop.io.erasurecode.coder.TestHHXORErasureCoder
            hadoop.security.TestKDiag



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863768/HADOOP-13200.03.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 18ad77128555 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 84a8848
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 13m 17s trunk passed +1 compile 16m 54s trunk passed +1 checkstyle 0m 37s trunk passed +1 mvnsite 1m 12s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 1m 40s trunk passed +1 javadoc 0m 51s trunk passed +1 mvninstall 0m 47s the patch passed +1 compile 15m 48s the patch passed +1 javac 15m 48s the patch passed -0 checkstyle 0m 35s hadoop-common-project/hadoop-common: The patch generated 30 new + 2 unchanged - 0 fixed = 32 total (was 2) +1 mvnsite 1m 2s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 35s the patch passed +1 javadoc 0m 49s the patch passed -1 unit 8m 9s hadoop-common in the patch failed. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 66m 37s Reason Tests Failed junit tests hadoop.io.erasurecode.coder.TestRSErasureCoder   hadoop.ha.TestZKFailoverController   hadoop.io.erasurecode.coder.TestHHXORErasureCoder   hadoop.security.TestKDiag Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863768/HADOOP-13200.03.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 18ad77128555 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 84a8848 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12116/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 13m 41s trunk passed
          +1 compile 17m 17s trunk passed
          +1 checkstyle 0m 39s trunk passed
          +1 mvnsite 1m 12s trunk passed
          +1 mvneclipse 0m 19s trunk passed
          +1 findbugs 1m 29s trunk passed
          +1 javadoc 0m 49s trunk passed
          +1 mvninstall 0m 39s the patch passed
          +1 compile 14m 39s the patch passed
          +1 javac 14m 39s the patch passed
          -0 checkstyle 0m 35s hadoop-common-project/hadoop-common: The patch generated 29 new + 2 unchanged - 0 fixed = 31 total (was 2)
          +1 mvnsite 1m 6s the patch passed
          +1 mvneclipse 0m 20s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 1m 45s the patch passed
          +1 javadoc 0m 51s the patch passed
          -1 unit 7m 15s hadoop-common in the patch failed.
          +1 asflicense 0m 45s The patch does not generate ASF License warnings.
          65m 39s



          Reason Tests
          Failed junit tests hadoop.security.TestShellBasedUnixGroupsMapping
            hadoop.io.erasurecode.coder.TestHHXORErasureCoder
            hadoop.io.erasurecode.coder.TestRSErasureCoder



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863772/HADOOP-13200.03.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 894eefe39809 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 654372d
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 13m 41s trunk passed +1 compile 17m 17s trunk passed +1 checkstyle 0m 39s trunk passed +1 mvnsite 1m 12s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 1m 29s trunk passed +1 javadoc 0m 49s trunk passed +1 mvninstall 0m 39s the patch passed +1 compile 14m 39s the patch passed +1 javac 14m 39s the patch passed -0 checkstyle 0m 35s hadoop-common-project/hadoop-common: The patch generated 29 new + 2 unchanged - 0 fixed = 31 total (was 2) +1 mvnsite 1m 6s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 45s the patch passed +1 javadoc 0m 51s the patch passed -1 unit 7m 15s hadoop-common in the patch failed. +1 asflicense 0m 45s The patch does not generate ASF License warnings. 65m 39s Reason Tests Failed junit tests hadoop.security.TestShellBasedUnixGroupsMapping   hadoop.io.erasurecode.coder.TestHHXORErasureCoder   hadoop.io.erasurecode.coder.TestRSErasureCoder Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863772/HADOOP-13200.03.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 894eefe39809 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 654372d Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12117/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          Going to move this one out to beta1 for tracking purposes, in case it doesn't get done by alpha3. If things progress quickly, happy to have it in alpha3 too.

          Show
          andrew.wang Andrew Wang added a comment - Going to move this one out to beta1 for tracking purposes, in case it doesn't get done by alpha3. If things progress quickly, happy to have it in alpha3 too.
          Hide
          timmyyao Tim Yao added a comment -

          Hi, Andrew. Thanks for reminding. I will do this task as soon as possible and try to make it ready for alpha3.

          Show
          timmyyao Tim Yao added a comment - Hi, Andrew. Thanks for reminding. I will do this task as soon as possible and try to make it ready for alpha3.
          Hide
          timmyyao Tim Yao added a comment -

          I have attached a new patch HADOOP-13200.04.patch. According to offline discussion with Kai, I have done some refactors to the patch, and also fixed the failure and code style problem in the old version.

          Show
          timmyyao Tim Yao added a comment - I have attached a new patch HADOOP-13200 .04.patch. According to offline discussion with Kai, I have done some refactors to the patch, and also fixed the failure and code style problem in the old version.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
          +1 mvninstall 13m 17s trunk passed
          +1 compile 16m 8s trunk passed
          +1 checkstyle 0m 35s trunk passed
          +1 mvnsite 1m 4s trunk passed
          +1 mvneclipse 0m 21s trunk passed
          +1 findbugs 1m 27s trunk passed
          +1 javadoc 0m 47s trunk passed
          +1 mvninstall 0m 38s the patch passed
          +1 compile 13m 44s the patch passed
          +1 javac 13m 44s the patch passed
          +1 checkstyle 0m 35s the patch passed
          +1 mvnsite 1m 1s the patch passed
          +1 mvneclipse 0m 20s the patch passed
          +1 whitespace 0m 1s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 1m 33s the patch passed
          +1 javadoc 0m 48s the patch passed
          -1 unit 8m 6s hadoop-common in the patch failed.
          +1 asflicense 0m 34s The patch does not generate ASF License warnings.
          63m 13s



          Reason Tests
          Failed junit tests hadoop.ha.TestZKFailoverController
            hadoop.security.TestKDiag



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863966/HADOOP-13200.04.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 0388b053034f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 8c81a16
          Default Java 1.8.0_121
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12123/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12123/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12123/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files. +1 mvninstall 13m 17s trunk passed +1 compile 16m 8s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 1m 4s trunk passed +1 mvneclipse 0m 21s trunk passed +1 findbugs 1m 27s trunk passed +1 javadoc 0m 47s trunk passed +1 mvninstall 0m 38s the patch passed +1 compile 13m 44s the patch passed +1 javac 13m 44s the patch passed +1 checkstyle 0m 35s the patch passed +1 mvnsite 1m 1s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 1s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 33s the patch passed +1 javadoc 0m 48s the patch passed -1 unit 8m 6s hadoop-common in the patch failed. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 63m 13s Reason Tests Failed junit tests hadoop.ha.TestZKFailoverController   hadoop.security.TestKDiag Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863966/HADOOP-13200.04.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 0388b053034f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8c81a16 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12123/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12123/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12123/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          timmyyao Tim Yao added a comment -

          A new patch HADOOP-13200.05.patch is attached according to the offline discussion with Kai to refine some details of the code and also to modify the related doc as some configurations of EC have been changed.

          Show
          timmyyao Tim Yao added a comment - A new patch HADOOP-13200 .05.patch is attached according to the offline discussion with Kai to refine some details of the code and also to modify the related doc as some configurations of EC have been changed.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 1s The patch appears to include 4 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 13m 18s trunk passed
          +1 compile 16m 7s trunk passed
          +1 checkstyle 1m 54s trunk passed
          +1 mvnsite 2m 3s trunk passed
          +1 mvneclipse 0m 42s trunk passed
          +1 findbugs 3m 19s trunk passed
          +1 javadoc 1m 39s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 30s the patch passed
          +1 compile 14m 7s the patch passed
          +1 javac 14m 7s the patch passed
          +1 checkstyle 1m 59s the patch passed
          +1 mvnsite 2m 7s the patch passed
          +1 mvneclipse 0m 48s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 3m 52s the patch passed
          +1 javadoc 1m 48s the patch passed
          -1 unit 8m 56s hadoop-common in the patch failed.
          -1 unit 80m 10s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 41s The patch does not generate ASF License warnings.
          180m 20s



          Reason Tests
          Failed junit tests hadoop.security.TestKDiag
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.datanode.TestDataNodeUUID
          Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2
            org.apache.hadoop.hdfs.server.blockmanagement.TestBlockStatsMXBean
            org.apache.hadoop.hdfs.server.namenode.ha.TestHASafeMode



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863993/HADOOP-13200.05.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux fd7d8e35cf77 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6b015d0
          Default Java 1.8.0_121
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 1s The patch appears to include 4 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 13m 18s trunk passed +1 compile 16m 7s trunk passed +1 checkstyle 1m 54s trunk passed +1 mvnsite 2m 3s trunk passed +1 mvneclipse 0m 42s trunk passed +1 findbugs 3m 19s trunk passed +1 javadoc 1m 39s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 30s the patch passed +1 compile 14m 7s the patch passed +1 javac 14m 7s the patch passed +1 checkstyle 1m 59s the patch passed +1 mvnsite 2m 7s the patch passed +1 mvneclipse 0m 48s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 52s the patch passed +1 javadoc 1m 48s the patch passed -1 unit 8m 56s hadoop-common in the patch failed. -1 unit 80m 10s hadoop-hdfs in the patch failed. +1 asflicense 0m 41s The patch does not generate ASF License warnings. 180m 20s Reason Tests Failed junit tests hadoop.security.TestKDiag   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestDataNodeUUID Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2   org.apache.hadoop.hdfs.server.blockmanagement.TestBlockStatsMXBean   org.apache.hadoop.hdfs.server.namenode.ha.TestHASafeMode Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863993/HADOOP-13200.05.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux fd7d8e35cf77 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6b015d0 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12124/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          drankye Kai Zheng added a comment -

          Thanks Tim Yao for working thru this and updating the patches. LGTM and my +1.

          I'm happy we can get rid of static things like follows,

          -  // Default coders for each codec names.
          -  public static final Map<String, String> DEFAULT_CODERS_MAP = ImmutableMap.of(
          -      "rs",         IO_ERASURECODE_CODEC_RS_RAWCODERS_DEFAULT,
          -      "rs-legacy",  IO_ERASURECODE_CODEC_RS_LEGACY_RAWCODERS_DEFAULT,
          -      "xor",        IO_ERASURECODE_CODEC_XOR_RAWCODERS_DEFAULT
          -  );
          

          by registries like the following, in In hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.erasurecode.rawcoder.RawErasureCoderFactory:

          +org.apache.hadoop.io.erasurecode.rawcoder.NativeRSRawErasureCoderFactory
          +org.apache.hadoop.io.erasurecode.rawcoder.NativeXORRawErasureCoderFactory
          +org.apache.hadoop.io.erasurecode.rawcoder.RSRawErasureCoderFactory
          +org.apache.hadoop.io.erasurecode.rawcoder.RSRawErasureCoderFactoryLegacy
          +org.apache.hadoop.io.erasurecode.rawcoder.XORRawErasureCoderFactory
          

          Andrew Wang, Wei-Chiu Chuang maybe you also want to take a look? If no other comments in a couple of days, I will commit it.

          Show
          drankye Kai Zheng added a comment - Thanks Tim Yao for working thru this and updating the patches. LGTM and my +1. I'm happy we can get rid of static things like follows, - // Default coders for each codec names. - public static final Map< String , String > DEFAULT_CODERS_MAP = ImmutableMap.of( - "rs" , IO_ERASURECODE_CODEC_RS_RAWCODERS_DEFAULT, - "rs-legacy" , IO_ERASURECODE_CODEC_RS_LEGACY_RAWCODERS_DEFAULT, - "xor" , IO_ERASURECODE_CODEC_XOR_RAWCODERS_DEFAULT - ); by registries like the following, in In hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.erasurecode.rawcoder.RawErasureCoderFactory: +org.apache.hadoop.io.erasurecode.rawcoder.NativeRSRawErasureCoderFactory +org.apache.hadoop.io.erasurecode.rawcoder.NativeXORRawErasureCoderFactory +org.apache.hadoop.io.erasurecode.rawcoder.RSRawErasureCoderFactory +org.apache.hadoop.io.erasurecode.rawcoder.RSRawErasureCoderFactoryLegacy +org.apache.hadoop.io.erasurecode.rawcoder.XORRawErasureCoderFactory Andrew Wang , Wei-Chiu Chuang maybe you also want to take a look? If no other comments in a couple of days, I will commit it.
          Hide
          andrew.wang Andrew Wang added a comment -

          Hi Tim Yao, thanks for working on this, looks great overall! Mostly just nits.

          One high-level comment is that I'd like to be precise with our use of terminology like codec, codec name, coder, raw coder, and raw coder factory. I made some more detailed comments based on my understanding, but would appreciate if you could confirm and make your own pass as well.

          Another high-level comment: should we restrict the names of third-party raw coders? Previously we had namespacing from the Java package name. Maybe we should do something like require classes that aren't in org.apache.hadoop to have a name that starts with "thirdparty_" and recommend that implementers add an additional namespace, e.g. "thirdparty_mycompany_". Maybe this should be handled as part of the pluggable coder work, with related docs.

          CodecRegistry:

          • Rather than the comment saying "put native coder first", please say why we do this. IIUC it's because this order is used as the default if the user does not specify a fallback order.
          • Nit: "code name" -> "codec name" in getCoderByName javadoc
          • Rename getCodecs to getCodecNames to match getCoderNames
          • Expanding the class Javadoc just a little bit would be good, explaining that it maps codec names to coder factories that implement that codec, and that we use ServiceLoader to find the factories.
          • Since we're using new Java, can skip the additional type declaration for generics on line 46 and 53.
          • Technically, isn't this a RawCoderFactoryRegistry, rather than a CodecRegistry?
          • Similar, shouldn't getCoders be named getFactories, and other references to Coders -> CoderFactory? We don't want to get this confused with ErasureCoder or RawErasureCoder/Decoder.

          CodecUtil:

          This line is split unnecessarily:

              fact = CodecRegistry.getInstance().getCoderByName(
                      codec, coderName);
          
          • createRawCoderFactory, rename codec parameter to codecName.
          • getCoderNames, rename function to getRawCoderNames, and rename codec param to codecName
          • createRawEncoderWithFallback and createRawDecoder: rename param codec to codecName, coderNames to rawCoderNames, coderName to rawCoderName

          RawErasureCoderFactory:

          • verifySchema is unused, remove?
          Show
          andrew.wang Andrew Wang added a comment - Hi Tim Yao , thanks for working on this, looks great overall! Mostly just nits. One high-level comment is that I'd like to be precise with our use of terminology like codec, codec name, coder, raw coder, and raw coder factory. I made some more detailed comments based on my understanding, but would appreciate if you could confirm and make your own pass as well. Another high-level comment: should we restrict the names of third-party raw coders? Previously we had namespacing from the Java package name. Maybe we should do something like require classes that aren't in org.apache.hadoop to have a name that starts with "thirdparty_" and recommend that implementers add an additional namespace, e.g. "thirdparty_mycompany_". Maybe this should be handled as part of the pluggable coder work, with related docs. CodecRegistry: Rather than the comment saying "put native coder first", please say why we do this. IIUC it's because this order is used as the default if the user does not specify a fallback order. Nit: "code name" -> "codec name" in getCoderByName javadoc Rename getCodecs to getCodecNames to match getCoderNames Expanding the class Javadoc just a little bit would be good, explaining that it maps codec names to coder factories that implement that codec, and that we use ServiceLoader to find the factories. Since we're using new Java, can skip the additional type declaration for generics on line 46 and 53. Technically, isn't this a RawCoderFactoryRegistry, rather than a CodecRegistry? Similar, shouldn't getCoders be named getFactories, and other references to Coders -> CoderFactory? We don't want to get this confused with ErasureCoder or RawErasureCoder/Decoder. CodecUtil: This line is split unnecessarily: fact = CodecRegistry.getInstance().getCoderByName( codec, coderName); createRawCoderFactory , rename codec parameter to codecName . getCoderNames , rename function to getRawCoderNames , and rename codec param to codecName createRawEncoderWithFallback and createRawDecoder : rename param codec to codecName , coderNames to rawCoderNames , coderName to rawCoderName RawErasureCoderFactory: verifySchema is unused, remove?
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Hello Kai Zheng

          Thanks for inviting me for comments, and thank Tim Yao for the multiple round of revisions.

          I made a quick pass of the patch, and I'd like to share a few comments, mostly on supportability and scalability:

          • what's the visibility of the new class CodecRegistry? I think it just needs package private, or at most Hadoop-only visibility (i.e. @InterfaceAudience.Private)
          • In the CodecRegistry constructor, please add debug level (or trace) log to print the codec put into the registry.
          • In the CodecRegistry constructor, could you make sure there is no existing coders in the list with the same coder name?
          • I am not familiar with Java 8 Lambda functions, but if I understand it correctly, CodecRegistry#getCoderNames returns a new String list every time it is called. Because this method is called whenever a DFSStripedOutputStream object is instantiated, it could cause extra heap usage over the long run.
          Show
          jojochuang Wei-Chiu Chuang added a comment - Hello Kai Zheng Thanks for inviting me for comments, and thank Tim Yao for the multiple round of revisions. I made a quick pass of the patch, and I'd like to share a few comments, mostly on supportability and scalability: what's the visibility of the new class CodecRegistry? I think it just needs package private, or at most Hadoop-only visibility (i.e. @InterfaceAudience.Private ) In the CodecRegistry constructor, please add debug level (or trace) log to print the codec put into the registry. In the CodecRegistry constructor, could you make sure there is no existing coders in the list with the same coder name? I am not familiar with Java 8 Lambda functions, but if I understand it correctly, CodecRegistry#getCoderNames returns a new String list every time it is called. Because this method is called whenever a DFSStripedOutputStream object is instantiated, it could cause extra heap usage over the long run.
          Hide
          timmyyao Tim Yao added a comment -

          Hi, Andrew Wang and Wei-Chiu Chuang
          Thanks a lot for your comments and suggestions, which are really helpful. I will give my own opinions on them.

          One high-level comment is that I'd like to be precise with our use of terminology like codec, codec name, coder, raw coder, and raw coder factory.

          It is good to be precise and I agree with most of your points. I’ll try the best to go thru the codes and make them consistent. On the other hand, I thought some bit of abstraction would be good to make the codes flexible and easier to evolve in future.

          Another high-level comment: should we restrict the names of third-party raw coders?

          This is a good suggestion for the purpose of the standardization of user-defined implementations. I just examined other pluggable components in Hadoop like CompressionCodec, and looks like they don’t use such restriction. But I do think we can do it in future development.

          Technically, isn't this a RawCoderFactoryRegistry, rather than a CodecRegistry?

          In CodecRegistry, all coders are registered and categorized by their codec. In high level, it manages all information of different codecs, so maybe CodecRegistry sounds more suitable and it will be easier for future extension of this component using this high-level name.

          I am not familiar with Java 8 Lambda functions, but if I understand it correctly, CodecRegistry#getCoderNames returns a new String list every time it is called. Because this method is called whenever a DFSStripedOutputStream object is instantiated, it could cause extra heap usage over the long run.

          This is a good point. I don’t worry about this much and guess JVM can manage the temp objects well, so I’d prefer to leave this simpler. If to avoid this and use another map managing coder names, it will incur some kinds of complexity overhead.

          For other comments that I did not mention, I think they are so reasonable that I fully agree with and I will modify accordingly, providing an update patch soon.

          Show
          timmyyao Tim Yao added a comment - Hi, Andrew Wang and Wei-Chiu Chuang Thanks a lot for your comments and suggestions, which are really helpful. I will give my own opinions on them. One high-level comment is that I'd like to be precise with our use of terminology like codec, codec name, coder, raw coder, and raw coder factory. It is good to be precise and I agree with most of your points. I’ll try the best to go thru the codes and make them consistent. On the other hand, I thought some bit of abstraction would be good to make the codes flexible and easier to evolve in future. Another high-level comment: should we restrict the names of third-party raw coders? This is a good suggestion for the purpose of the standardization of user-defined implementations. I just examined other pluggable components in Hadoop like CompressionCodec, and looks like they don’t use such restriction. But I do think we can do it in future development. Technically, isn't this a RawCoderFactoryRegistry, rather than a CodecRegistry? In CodecRegistry, all coders are registered and categorized by their codec. In high level, it manages all information of different codecs, so maybe CodecRegistry sounds more suitable and it will be easier for future extension of this component using this high-level name. I am not familiar with Java 8 Lambda functions, but if I understand it correctly, CodecRegistry#getCoderNames returns a new String list every time it is called. Because this method is called whenever a DFSStripedOutputStream object is instantiated, it could cause extra heap usage over the long run. This is a good point. I don’t worry about this much and guess JVM can manage the temp objects well, so I’d prefer to leave this simpler. If to avoid this and use another map managing coder names, it will incur some kinds of complexity overhead. For other comments that I did not mention, I think they are so reasonable that I fully agree with and I will modify accordingly, providing an update patch soon.
          Hide
          drankye Kai Zheng added a comment -

          As now codecs are dynamically identified and loaded along with its raw coder implementations, for users to use them when define erasure coding policies, a CLI cmd would be needed to list these codecs. The cmd would simply list the keys of the map maintained by CodecRegistry. I suggest we do this separately.

          Show
          drankye Kai Zheng added a comment - As now codecs are dynamically identified and loaded along with its raw coder implementations, for users to use them when define erasure coding policies, a CLI cmd would be needed to list these codecs. The cmd would simply list the keys of the map maintained by CodecRegistry . I suggest we do this separately.
          Hide
          timmyyao Tim Yao added a comment -

          I attached a new patch HADOOP-13200.06.patch according to the above comments.

          Show
          timmyyao Tim Yao added a comment - I attached a new patch HADOOP-13200 .06.patch according to the above comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 15m 7s trunk passed
          +1 compile 18m 5s trunk passed
          +1 checkstyle 2m 0s trunk passed
          +1 mvnsite 2m 23s trunk passed
          +1 mvneclipse 0m 42s trunk passed
          -1 findbugs 1m 44s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings.
          -1 findbugs 2m 2s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings.
          +1 javadoc 1m 49s trunk passed
          0 mvndep 0m 23s Maven dependency ordering for patch
          +1 mvninstall 1m 47s the patch passed
          +1 compile 16m 25s the patch passed
          +1 javac 16m 25s the patch passed
          +1 checkstyle 2m 8s the patch passed
          +1 mvnsite 2m 25s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 4m 5s the patch passed
          +1 javadoc 1m 59s the patch passed
          -1 unit 9m 9s hadoop-common in the patch failed.
          -1 unit 66m 1s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 42s The patch does not generate ASF License warnings.
          175m 3s



          Reason Tests
          Failed junit tests hadoop.security.TestKDiag
            hadoop.hdfs.server.datanode.metrics.TestDataNodeOutlierDetectionViaMetrics
            hadoop.hdfs.TestMaintenanceState
            hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
            hadoop.hdfs.TestCrcCorruption



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864457/HADOOP-13200.06.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 69be201ec9d4 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b080338
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 15m 7s trunk passed +1 compile 18m 5s trunk passed +1 checkstyle 2m 0s trunk passed +1 mvnsite 2m 23s trunk passed +1 mvneclipse 0m 42s trunk passed -1 findbugs 1m 44s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings. -1 findbugs 2m 2s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings. +1 javadoc 1m 49s trunk passed 0 mvndep 0m 23s Maven dependency ordering for patch +1 mvninstall 1m 47s the patch passed +1 compile 16m 25s the patch passed +1 javac 16m 25s the patch passed +1 checkstyle 2m 8s the patch passed +1 mvnsite 2m 25s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 4m 5s the patch passed +1 javadoc 1m 59s the patch passed -1 unit 9m 9s hadoop-common in the patch failed. -1 unit 66m 1s hadoop-hdfs in the patch failed. +1 asflicense 0m 42s The patch does not generate ASF License warnings. 175m 3s Reason Tests Failed junit tests hadoop.security.TestKDiag   hadoop.hdfs.server.datanode.metrics.TestDataNodeOutlierDetectionViaMetrics   hadoop.hdfs.TestMaintenanceState   hadoop.hdfs.server.namenode.ha.TestPipelinesFailover   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting   hadoop.hdfs.TestCrcCorruption Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864457/HADOOP-13200.06.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 69be201ec9d4 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b080338 Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12146/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          This is a good point. I don’t worry about this much and guess JVM can manage the temp objects well, so I’d prefer to leave this simpler. If to avoid this and use another map managing coder names, it will incur some kinds of complexity overhead.

          Thanks for the comment.
          This might be true for other projects, but untrue for Hadoop, especially in NameNodes.
          For DataNodes, this patch returns a new array every time a connection is established. Given that a DataNode can have thousands of concurrent client connections on a busy cluster, adding this extra overhead is not a good idea. Plus, this array will not be updated after initialization. I think we can do a better job than that. If you think the improvement incurs extra code complexity, I don't mind filing a new jira to improve this.

          Show
          jojochuang Wei-Chiu Chuang added a comment - This is a good point. I don’t worry about this much and guess JVM can manage the temp objects well, so I’d prefer to leave this simpler. If to avoid this and use another map managing coder names, it will incur some kinds of complexity overhead. Thanks for the comment. This might be true for other projects, but untrue for Hadoop, especially in NameNodes. For DataNodes, this patch returns a new array every time a connection is established. Given that a DataNode can have thousands of concurrent client connections on a busy cluster, adding this extra overhead is not a good idea. Plus, this array will not be updated after initialization. I think we can do a better job than that. If you think the improvement incurs extra code complexity, I don't mind filing a new jira to improve this.
          Hide
          jojochuang Wei-Chiu Chuang added a comment - - edited

          I saw a few nits in the 06 patch:

          • Could you switch to slf4j? That way you can avoid LOG.isDebugEnabled() and can use curl based parameterization.
          • Would it be possible to add a test for the case where two coders register the same name?
          • The following code
            " cannot be registered because its coder name " + coderFactory
            

            shouldn't it be coderFactory.getCoderName()?

          • Still in CodecRegistry constructor. I think you want to continue instead of break if a coder has conflict. Otherwise you would throw an exception instead of just log an error message.
          Show
          jojochuang Wei-Chiu Chuang added a comment - - edited I saw a few nits in the 06 patch: Could you switch to slf4j? That way you can avoid LOG.isDebugEnabled() and can use curl based parameterization. Would it be possible to add a test for the case where two coders register the same name? The following code " cannot be registered because its coder name " + coderFactory shouldn't it be coderFactory.getCoderName()? Still in CodecRegistry constructor. I think you want to continue instead of break if a coder has conflict. Otherwise you would throw an exception instead of just log an error message.
          Hide
          timmyyao Tim Yao added a comment -

          Thanks Wei-Chiu Chuang for your comments.

          Given that a DataNode can have thousands of concurrent client connections on a busy cluster, adding this extra overhead is not a good idea. Plus, this array will not be updated after initialization. I think we can do a better job than that.

          I think this is a quite important reason for the modification of this part of code. I will improve it using another array of names to avoid the extra overhead.

          Still in CodecRegistry constructor. I think you want to continue instead of break if a coder has conflict. Otherwise you would throw an exception instead of just log an error message.

          I am sorry I don't quite get the point. I use break when finding whether the coder name has conflict. If the same name has been detected, an error message will be printed and loop for the coder name detection will be ended. This coder factory will then be ignored and the constructor will still continue to register the remaining coder factories.

          And I will take the other advises and apply a new patch accordingly.

          Show
          timmyyao Tim Yao added a comment - Thanks Wei-Chiu Chuang for your comments. Given that a DataNode can have thousands of concurrent client connections on a busy cluster, adding this extra overhead is not a good idea. Plus, this array will not be updated after initialization. I think we can do a better job than that. I think this is a quite important reason for the modification of this part of code. I will improve it using another array of names to avoid the extra overhead. Still in CodecRegistry constructor. I think you want to continue instead of break if a coder has conflict. Otherwise you would throw an exception instead of just log an error message. I am sorry I don't quite get the point. I use break when finding whether the coder name has conflict. If the same name has been detected, an error message will be printed and loop for the coder name detection will be ended. This coder factory will then be ignored and the constructor will still continue to register the remaining coder factories. And I will take the other advises and apply a new patch accordingly.
          Hide
          timmyyao Tim Yao added a comment -

          The new patch HADOOP-13200.07.patch is modified according to the above discussion. The major modification is using another map to manage coder names, and the other nits are also fixed accordingly.

          Show
          timmyyao Tim Yao added a comment - The new patch HADOOP-13200 .07.patch is modified according to the above discussion. The major modification is using another map to manage coder names, and the other nits are also fixed accordingly.
          Hide
          timmyyao Tim Yao added a comment -

          I did some modifications in HADOOP-13200.08.patch on the unit test for CodecRegistry.

          Show
          timmyyao Tim Yao added a comment - I did some modifications in HADOOP-13200 .08.patch on the unit test for CodecRegistry.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Hi Tim Yao
          Thanks a lot for the new rev. It looks almost good, and I am +1 after just three nits:

          • Please add a @VisibleForTesting annotation to the method updateCoders
          • Please remove this comment
            //userDefinedFactories.add(new RSUserDefinedFactory());
            
          • Since we switched to slf4j, could we also use curl style parameters? Note that without using parameterized logging, this would incur additional parameter construction cost, which is not a good idea for debug messages. E.g.
            	            LOG.debug("Codec registered: codec = {}, coder = {}",
            	            coderFactory.getCodecName(),coderFactory.getCoderName());
            

          The user doc is not that obvious to first time users. But let's defer that to a new jira.

          Would any one else like to comment? Now's a good time

          Show
          jojochuang Wei-Chiu Chuang added a comment - Hi Tim Yao Thanks a lot for the new rev. It looks almost good, and I am +1 after just three nits: Please add a @VisibleForTesting annotation to the method updateCoders Please remove this comment //userDefinedFactories.add( new RSUserDefinedFactory()); Since we switched to slf4j, could we also use curl style parameters? Note that without using parameterized logging, this would incur additional parameter construction cost, which is not a good idea for debug messages. E.g. LOG.debug( "Codec registered: codec = {}, coder = {}" , coderFactory.getCodecName(),coderFactory.getCoderName()); The user doc is not that obvious to first time users. But let's defer that to a new jira. Would any one else like to comment? Now's a good time
          Hide
          timmyyao Tim Yao added a comment -

          Thanks Wei-Chiu Chuang a lot. I have attached a new rev HADOOP-13200.09.patch which fixed the above three nits.

          Show
          timmyyao Tim Yao added a comment - Thanks Wei-Chiu Chuang a lot. I have attached a new rev HADOOP-13200 .09.patch which fixed the above three nits.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          +1 pending Jenkins.

          Show
          jojochuang Wei-Chiu Chuang added a comment - +1 pending Jenkins.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Seems Jenkins was never triggered. I just triggered it manually. Monitor the job status here: https://builds.apache.org/job/PreCommit-HADOOP-Build/12180/

          Show
          jojochuang Wei-Chiu Chuang added a comment - Seems Jenkins was never triggered. I just triggered it manually. Monitor the job status here: https://builds.apache.org/job/PreCommit-HADOOP-Build/12180/
          Hide
          timmyyao Tim Yao added a comment -

          It failed and there seems some error with the environment?

          fatal: unable to access 'https://git-wip-us.apache.org/repos/asf/hadoop.git/': server certificate verification failed. CAfile: /etc/ssl/certs/ca-certificates.crt CRLfile: none
          ERROR: git pull is failing

          Show
          timmyyao Tim Yao added a comment - It failed and there seems some error with the environment? fatal: unable to access 'https://git-wip-us.apache.org/repos/asf/hadoop.git/': server certificate verification failed. CAfile: /etc/ssl/certs/ca-certificates.crt CRLfile: none ERROR: git pull is failing
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          There was an infra issue. Looks like it was just resolved. I triggered another build for you: https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/

          Show
          jojochuang Wei-Chiu Chuang added a comment - There was an infra issue. Looks like it was just resolved. I triggered another build for you: https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
          0 mvndep 0m 13s Maven dependency ordering for branch
          +1 mvninstall 13m 22s trunk passed
          +1 compile 15m 49s trunk passed
          +1 checkstyle 1m 54s trunk passed
          +1 mvnsite 2m 9s trunk passed
          +1 mvneclipse 0m 42s trunk passed
          -1 findbugs 1m 24s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings.
          -1 findbugs 1m 46s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings.
          +1 javadoc 1m 41s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 40s the patch passed
          +1 compile 15m 8s the patch passed
          +1 javac 15m 8s the patch passed
          -0 checkstyle 2m 5s root: The patch generated 1 new + 22 unchanged - 0 fixed = 23 total (was 22)
          +1 mvnsite 2m 8s the patch passed
          +1 mvneclipse 0m 48s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 3m 43s the patch passed
          +1 javadoc 1m 51s the patch passed
          -1 unit 9m 6s hadoop-common in the patch failed.
          -1 unit 68m 26s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 43s The patch does not generate ASF License warnings.
          169m 44s



          Reason Tests
          Failed junit tests hadoop.ha.TestZKFailoverController
            hadoop.security.TestKDiag
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.TestDFSClientRetries
            hadoop.hdfs.server.datanode.TestDataNodeUUID
            hadoop.hdfs.TestMaintenanceState
            hadoop.hdfs.server.namenode.ha.TestPipelinesFailover



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864876/HADOOP-13200.09.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 102092e31225 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 2f73396
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files. 0 mvndep 0m 13s Maven dependency ordering for branch +1 mvninstall 13m 22s trunk passed +1 compile 15m 49s trunk passed +1 checkstyle 1m 54s trunk passed +1 mvnsite 2m 9s trunk passed +1 mvneclipse 0m 42s trunk passed -1 findbugs 1m 24s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings. -1 findbugs 1m 46s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings. +1 javadoc 1m 41s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 40s the patch passed +1 compile 15m 8s the patch passed +1 javac 15m 8s the patch passed -0 checkstyle 2m 5s root: The patch generated 1 new + 22 unchanged - 0 fixed = 23 total (was 22) +1 mvnsite 2m 8s the patch passed +1 mvneclipse 0m 48s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 43s the patch passed +1 javadoc 1m 51s the patch passed -1 unit 9m 6s hadoop-common in the patch failed. -1 unit 68m 26s hadoop-hdfs in the patch failed. +1 asflicense 0m 43s The patch does not generate ASF License warnings. 169m 44s Reason Tests Failed junit tests hadoop.ha.TestZKFailoverController   hadoop.security.TestKDiag   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.TestDFSClientRetries   hadoop.hdfs.server.datanode.TestDataNodeUUID   hadoop.hdfs.TestMaintenanceState   hadoop.hdfs.server.namenode.ha.TestPipelinesFailover Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864876/HADOOP-13200.09.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 102092e31225 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2f73396 Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12191/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          timmyyao Tim Yao added a comment -

          There is one nit related to my patch.

          ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java:21:import org.apache.hadoop.io.erasurecode.ECSchema;:8: Unused import - org.apache.hadoop.io.erasurecode.ECSchema. [UnusedImports]

          I have fixed it in the new rev. And the others are not related to this patch.

          Show
          timmyyao Tim Yao added a comment - There is one nit related to my patch. ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java:21:import org.apache.hadoop.io.erasurecode.ECSchema;:8: Unused import - org.apache.hadoop.io.erasurecode.ECSchema. [UnusedImports] I have fixed it in the new rev. And the others are not related to this patch.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Triggering the build again. If it doesn't come back I'll commit patch 10 anyway.

          Show
          jojochuang Wei-Chiu Chuang added a comment - Triggering the build again. If it doesn't come back I'll commit patch 10 anyway.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
          0 mvndep 0m 13s Maven dependency ordering for branch
          +1 mvninstall 13m 18s trunk passed
          +1 compile 15m 48s trunk passed
          +1 checkstyle 1m 55s trunk passed
          +1 mvnsite 2m 4s trunk passed
          +1 mvneclipse 0m 42s trunk passed
          -1 findbugs 1m 25s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings.
          -1 findbugs 1m 47s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings.
          +1 javadoc 1m 38s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 29s the patch passed
          +1 compile 14m 37s the patch passed
          +1 javac 14m 37s the patch passed
          +1 checkstyle 1m 55s the patch passed
          +1 mvnsite 2m 8s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 3m 35s the patch passed
          +1 javadoc 1m 47s the patch passed
          -1 unit 7m 55s hadoop-common in the patch failed.
          -1 unit 65m 10s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 42s The patch does not generate ASF License warnings.
          164m 15s



          Reason Tests
          Failed junit tests hadoop.security.TestRaceWhenRelogin
            hadoop.security.TestKDiag
            hadoop.hdfs.server.namenode.ha.TestPipelinesFailover



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865134/HADOOP-13200.10.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux df36e8bfae02 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 93fa48f
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files. 0 mvndep 0m 13s Maven dependency ordering for branch +1 mvninstall 13m 18s trunk passed +1 compile 15m 48s trunk passed +1 checkstyle 1m 55s trunk passed +1 mvnsite 2m 4s trunk passed +1 mvneclipse 0m 42s trunk passed -1 findbugs 1m 25s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings. -1 findbugs 1m 47s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings. +1 javadoc 1m 38s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 29s the patch passed +1 compile 14m 37s the patch passed +1 javac 14m 37s the patch passed +1 checkstyle 1m 55s the patch passed +1 mvnsite 2m 8s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 35s the patch passed +1 javadoc 1m 47s the patch passed -1 unit 7m 55s hadoop-common in the patch failed. -1 unit 65m 10s hadoop-hdfs in the patch failed. +1 asflicense 0m 42s The patch does not generate ASF License warnings. 164m 15s Reason Tests Failed junit tests hadoop.security.TestRaceWhenRelogin   hadoop.security.TestKDiag   hadoop.hdfs.server.namenode.ha.TestPipelinesFailover Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865134/HADOOP-13200.10.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux df36e8bfae02 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 93fa48f Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12195/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          TestPipelinesFailover failed consistently in my local machine as well.
          I don't think it's related to this patch, but would you please double check?

          Show
          jojochuang Wei-Chiu Chuang added a comment - TestPipelinesFailover failed consistently in my local machine as well. I don't think it's related to this patch, but would you please double check?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 29s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
          0 mvndep 1m 24s Maven dependency ordering for branch
          +1 mvninstall 14m 45s trunk passed
          +1 compile 16m 47s trunk passed
          +1 checkstyle 1m 57s trunk passed
          +1 mvnsite 2m 19s trunk passed
          +1 mvneclipse 0m 43s trunk passed
          -1 findbugs 1m 34s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings.
          -1 findbugs 1m 48s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings.
          +1 javadoc 1m 40s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 44s the patch passed
          +1 compile 15m 42s the patch passed
          +1 javac 15m 42s the patch passed
          +1 checkstyle 1m 55s the patch passed
          +1 mvnsite 2m 10s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 4m 7s the patch passed
          +1 javadoc 1m 45s the patch passed
          -1 unit 8m 51s hadoop-common in the patch failed.
          -1 unit 66m 54s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 44s The patch does not generate ASF License warnings.
          172m 34s



          Reason Tests
          Failed junit tests hadoop.security.TestShellBasedUnixGroupsMapping
            hadoop.security.TestKDiag
            hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865134/HADOOP-13200.10.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 197a3a12b82d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 8b5f2c3
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 29s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files. 0 mvndep 1m 24s Maven dependency ordering for branch +1 mvninstall 14m 45s trunk passed +1 compile 16m 47s trunk passed +1 checkstyle 1m 57s trunk passed +1 mvnsite 2m 19s trunk passed +1 mvneclipse 0m 43s trunk passed -1 findbugs 1m 34s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings. -1 findbugs 1m 48s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings. +1 javadoc 1m 40s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 44s the patch passed +1 compile 15m 42s the patch passed +1 javac 15m 42s the patch passed +1 checkstyle 1m 55s the patch passed +1 mvnsite 2m 10s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 4m 7s the patch passed +1 javadoc 1m 45s the patch passed -1 unit 8m 51s hadoop-common in the patch failed. -1 unit 66m 54s hadoop-hdfs in the patch failed. +1 asflicense 0m 44s The patch does not generate ASF License warnings. 172m 34s Reason Tests Failed junit tests hadoop.security.TestShellBasedUnixGroupsMapping   hadoop.security.TestKDiag   hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865134/HADOOP-13200.10.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 197a3a12b82d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8b5f2c3 Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12198/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          timmyyao Tim Yao added a comment - - edited

          I have double checked the code and don't think it's related to this patch either, as CodecUtil is never used in this class.
          By the way, I examined some other bugs when checking all the usages of CodecUtil. As the configuration of erasure code has been changed, some configurations should be changed accordingly in unit tests. All those bugs lie in the codes like the following:

          if (ErasureCodeNative.isNativeCodeLoaded()) {
          conf.set(
          CodecUtil.IO_ERASURECODE_CODEC_RS_RAWCODERS_KEY,
          NativeRSRawErasureCoderFactory.class.getCanonicalName());
          }

          The configuration is done when native code is loaded, so currently there is no error displayed. I will correct them immediately.

          Show
          timmyyao Tim Yao added a comment - - edited I have double checked the code and don't think it's related to this patch either, as CodecUtil is never used in this class. By the way, I examined some other bugs when checking all the usages of CodecUtil. As the configuration of erasure code has been changed, some configurations should be changed accordingly in unit tests. All those bugs lie in the codes like the following: if (ErasureCodeNative.isNativeCodeLoaded()) { conf.set( CodecUtil.IO_ERASURECODE_CODEC_RS_RAWCODERS_KEY, NativeRSRawErasureCoderFactory.class.getCanonicalName()); } The configuration is done when native code is loaded, so currently there is no error displayed. I will correct them immediately.
          Hide
          timmyyao Tim Yao added a comment - - edited

          In the new patch HADOOP-13200.11.patch, the above configuration errors are all fixed. Only simple modifications are done by replacing NativeRSRawErasureCoderFactory.class.getCanonicalName() with NativeRSRawErasureCoderFactory.CODER_NAME, which shall not cause any additional problems.

          Show
          timmyyao Tim Yao added a comment - - edited In the new patch HADOOP-13200 .11.patch, the above configuration errors are all fixed. Only simple modifications are done by replacing NativeRSRawErasureCoderFactory.class.getCanonicalName() with NativeRSRawErasureCoderFactory.CODER_NAME, which shall not cause any additional problems.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Okay... I reverted this patch and I am still seeing the same error. It looks like the test has been failing since a few days ago.

          Show
          jojochuang Wei-Chiu Chuang added a comment - Okay... I reverted this patch and I am still seeing the same error. It looks like the test has been failing since a few days ago.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 9 new or modified test files.
          0 mvndep 0m 13s Maven dependency ordering for branch
          +1 mvninstall 13m 56s trunk passed
          +1 compile 16m 51s trunk passed
          +1 checkstyle 1m 59s trunk passed
          +1 mvnsite 2m 24s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          -1 findbugs 1m 41s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings.
          -1 findbugs 1m 59s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings.
          +1 javadoc 1m 43s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 52s the patch passed
          +1 compile 15m 7s the patch passed
          +1 javac 15m 7s the patch passed
          +1 checkstyle 1m 55s the patch passed
          +1 mvnsite 2m 8s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 3m 34s the patch passed
          +1 javadoc 1m 45s the patch passed
          -1 unit 7m 54s hadoop-common in the patch failed.
          -1 unit 63m 57s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 43s The patch does not generate ASF License warnings.
          166m 19s



          Reason Tests
          Failed junit tests hadoop.security.TestKDiag
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-13200
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865267/HADOOP-13200.11.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 6a1a1966e35d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 28eb2aa
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 9 new or modified test files. 0 mvndep 0m 13s Maven dependency ordering for branch +1 mvninstall 13m 56s trunk passed +1 compile 16m 51s trunk passed +1 checkstyle 1m 59s trunk passed +1 mvnsite 2m 24s trunk passed +1 mvneclipse 0m 39s trunk passed -1 findbugs 1m 41s hadoop-common-project/hadoop-common in trunk has 17 extant Findbugs warnings. -1 findbugs 1m 59s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings. +1 javadoc 1m 43s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 52s the patch passed +1 compile 15m 7s the patch passed +1 javac 15m 7s the patch passed +1 checkstyle 1m 55s the patch passed +1 mvnsite 2m 8s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 34s the patch passed +1 javadoc 1m 45s the patch passed -1 unit 7m 54s hadoop-common in the patch failed. -1 unit 63m 57s hadoop-hdfs in the patch failed. +1 asflicense 0m 43s The patch does not generate ASF License warnings. 166m 19s Reason Tests Failed junit tests hadoop.security.TestKDiag   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-13200 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865267/HADOOP-13200.11.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 6a1a1966e35d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 28eb2aa Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12199/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          +1 LGTM.
          findbugs warnings are unrelated. Failed tests are unrelated and not reproducible on my local machine.

          Will commit soon.

          Show
          jojochuang Wei-Chiu Chuang added a comment - +1 LGTM. findbugs warnings are unrelated. Failed tests are unrelated and not reproducible on my local machine. Will commit soon.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #11641 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11641/)
          HADOOP-13200. Implement customizable and configurable erasure coders. (weichiu: rev 872088c6e7e78534a8ffd1ea6e57b86faea3d6e1)

          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestRSErasureCoder.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSLegacyRawErasureCoderFactory.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStream.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecUtil.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestHHXORErasureCoder.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/ErasureCodeConstants.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/DummyRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/XORRawErasureCoderFactory.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRawCoderMapping.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #11641 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11641/ ) HADOOP-13200 . Implement customizable and configurable erasure coders. (weichiu: rev 872088c6e7e78534a8ffd1ea6e57b86faea3d6e1) (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestRSErasureCoder.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSLegacyRawErasureCoderFactory.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStream.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecUtil.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestHHXORErasureCoder.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/ErasureCodeConstants.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/DummyRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/XORRawErasureCoderFactory.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRawCoderMapping.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Committed rev 11 patch to trunk.

          Thanks very much to Tim Yao for your persistent revision, Kai Zheng for your initial reviews, and inputs from Kai Sasaki and Andrew Wang.

          Added incompatible change flag because it changed valid configuration values.

          Tim Yao would you please also update release note to summarize this work? Thanks.

          Show
          jojochuang Wei-Chiu Chuang added a comment - Committed rev 11 patch to trunk. Thanks very much to Tim Yao for your persistent revision, Kai Zheng for your initial reviews, and inputs from Kai Sasaki and Andrew Wang . Added incompatible change flag because it changed valid configuration values. Tim Yao would you please also update release note to summarize this work? Thanks.
          Hide
          djp Junping Du added a comment -

          Trunk build get broken ...
          Can we make sure build pass before we commit the patch?

          Show
          djp Junping Du added a comment - Trunk build get broken ... Can we make sure build pass before we commit the patch?
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Oh I am sorry. It does compile but I forgot to add new files into commit.

          Show
          jojochuang Wei-Chiu Chuang added a comment - Oh I am sorry. It does compile but I forgot to add new files into commit.
          Hide
          djp Junping Du added a comment -

          I see. No worry. We all make same mistake before.

          Show
          djp Junping Du added a comment - I see. No worry. We all make same mistake before.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Added new files, and verified it compiles.

          Thanks to Junping Du for catching the issue quickly!

          Show
          jojochuang Wei-Chiu Chuang added a comment - Added new files, and verified it compiles. Thanks to Junping Du for catching the issue quickly!
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11642 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11642/)
          Revert "HADOOP-13200. Implement customizable and configurable erasure (weichiu: rev ddaeb3e4979338551573285f219a2d361f502950)

          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/XORRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecUtil.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawErasureCoderFactory.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStream.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestRSErasureCoder.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRawCoderMapping.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/DummyRawErasureCoderFactory.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSLegacyRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/ErasureCodeConstants.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestHHXORErasureCoder.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11642 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11642/ ) Revert " HADOOP-13200 . Implement customizable and configurable erasure (weichiu: rev ddaeb3e4979338551573285f219a2d361f502950) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/XORRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecUtil.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawErasureCoderFactory.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStream.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestRSErasureCoder.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRawCoderMapping.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/DummyRawErasureCoderFactory.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSLegacyRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/ErasureCodeConstants.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestHHXORErasureCoder.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11643 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11643/)
          HADOOP-13200. Implement customizable and configurable erasure coders. (weichiu: rev bbf8cac14d8b1a0e919b57cb7f081edce45acd5b)

          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/ErasureCodeConstants.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecUtil.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSRawErasureCoderFactory.java
          • (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRegistry.java
          • (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecRegistry.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestRSErasureCoder.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/DummyRawErasureCoderFactory.java
          • (add) hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.erasurecode.rawcoder.RawErasureCoderFactory
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/XORRawErasureCoderFactory.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSLegacyRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRawCoderMapping.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawErasureCoderFactory.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestHHXORErasureCoder.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11643 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11643/ ) HADOOP-13200 . Implement customizable and configurable erasure coders. (weichiu: rev bbf8cac14d8b1a0e919b57cb7f081edce45acd5b) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/ErasureCodeConstants.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecUtil.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSRawErasureCoderFactory.java (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRegistry.java (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecRegistry.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestRSErasureCoder.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/DummyRawErasureCoderFactory.java (add) hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.erasurecode.rawcoder.RawErasureCoderFactory (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/XORRawErasureCoderFactory.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/RSLegacyRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCodecRawCoderMapping.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawErasureCoderFactory.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestHHXORErasureCoder.java
          Hide
          timmyyao Tim Yao added a comment -

          Thanks all for reviewing and offering so many helpful suggestions. I have updated the release note.

          Show
          timmyyao Tim Yao added a comment - Thanks all for reviewing and offering so many helpful suggestions. I have updated the release note.

            People

            • Assignee:
              timmyyao Tim Yao
              Reporter:
              drankye Kai Zheng
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development