You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by Mark Bean <ma...@gmail.com> on 2022/10/08 16:57:01 UTC

dealing with a breaking change

I am working on NIFI-10243 [1]. The goal is to allow ControlRate to
throttle based on both data rate and FlowFile count. If either rate is
exceeded, throttling occurs.

Currently, throttling occurs in only one mode. Therefore, a single
property, Maximum Rate, is overloaded to accept either a size limit or a
count limit. Changing how this property is used could become a breaking
change. However, keeping backward compatibility makes the property usage
less logical.

It makes good sense to have two properties, one for data rate, e.g. "1 MB",
and one for count rate, e.g. "10". Unfortunately, this implementation would
break current instantiations of the processor since an integer value in the
existing attribute would not be accepted any longer.

It is possible to keep the functionality the same and introduce a new
"Maximum Count Rate" property which is to be used _only_ when the Rate
Control Criteria is the new 'data rate and flowfile count'. I don't feel
this makes the processor property usage easy to understand though. The
flowfile count rate would be provided in the existing Maximum Rate property
if the criteria is 'flowfile count', but it would have to be specified in a
new property if the criteria is 'data rate and flowfile count'. This is
inconsistent and confusing.

Perhaps, "Maximum Rate" property could be overloaded to accept a byte
value, an integer value or a comma-separated list of both, depending on the
selected rate control criteria. It's clunky, but could work.

Another alternative to avoid a breaking change is to introduce a new
processor, ExtendedControlRate, which allows for the new Rate Control
Criteria and redefined rate properties.

So, the choices are:
1) Breaking change aligning properties with their purpose in an easy to
understand manner
2) Backward compatible where flowfile count is specified in different
properties depending on rate control criteria
3) Backward compatible where Maximum Rate accepts a string, an integer or
both (comma-separated)
4) Introduce a new processor

I think option 1 makes the most sense, but I'm concerned about the breaking
nature of the approach. Looking for suggestions on how to proceed.

[1] https://issues.apache.org/jira/browse/NIFI-10243

Re: dealing with a breaking change

Posted by Mark Payne <ma...@hotmail.com>.
I think the best solution here is definitely to make use of dependent properties. We would leave the existing “Maximum Rate” property as-is, except make it
depend on the Rate Control Criteria being one of the currently existing values. And then we’d add a new allowable value for the Rate Control Criteria. So the
Property Descriptors would look like this:


public static final PropertyDescriptor RATE_CONTROL_CRITERIA = new PropertyDescriptor.Builder()
        .name("Rate Control Criteria")
        .description("Indicates the criteria that is used to control the throughput rate. Changing this value resets the rate counters.")
        .required(true)
        .allowableValues(DATA_RATE_VALUE, FLOWFILE_RATE_VALUE, ATTRIBUTE_RATE_VALUE, DATA_OR_FLOWFILE_RATE) // DATA_OR_FLOWFILE_RATE is newly added.
        .defaultValue(DATA_RATE)
        .build();
public static final PropertyDescriptor MAX_RATE = new PropertyDescriptor.Builder()  // Existing Property
    .name("Maximum Rate")
    .description("The maximum rate at which data should pass through this processor. The format of this property is expected to be a "
            + "positive integer, or a Data Size (such as '1 MB') if Rate Control Criteria is set to 'data rate'.")
    .required(true)
    .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) // validated in customValidate b/c dependent on Rate Control Criteria
    .dependsOn(RATE_CONTROL_CRITERIA, DATA_RATE_VALUE, FLOWFILE_RATE_VALUE, ATTRIBUTE_RATE_VALUE)
    .build();
public static final PropertyDescriptor MAX_DATA_RATE = new PropertyDescriptor.Builder() // Newly Added Property
    .name("Maximum Data Rate")
    .description("The maximum amount of data to allow through during the specified Time Duration.")
    .required(true)
    .addValidator(StandardValidators.DATA_SIZE_VALIDATOR)
    .dependsOn(RATE_CONTROL_CRITERIA, DATA_OR_FLOWFILE_RATE)
    .build();
public static final PropertyDescriptor MAX_FLOWFILE_RATE = new PropertyDescriptor.Builder()  // Newly Added Property
    .name("Maximum FlowFile Rate")
    .description("The maximum number of FlowFiles to allow through during the specified Time Duration.")
    .required(true)
    .addValidator(StandardValidators.POSITIVE_INTEGER_VALIDATOR)
    .dependsOn(RATE_CONTROL_CRITERIA, DATA_OR_FLOWFILE_RATE)
    .build();

This way, everything remains backward compatible. If the new value is selected, in order to throttle on both, then the user would see two new properties and the old “Maximum Rate” property would disappear from the UI.
So the UX makes sense, and we remain backward compatible, without introducing any new processor.

Thanks
-Mark


On Oct 8, 2022, at 7:00 PM, Mark Bean <ma...@gmail.com>> wrote:

There was a reason I left "new processor" as the last option. In my
opinion, it's the least desirable - but honestly, it's the easiest. Still...

Deprecating the property isn't really appropriate here because the property
remains. It's just that its behavior (allowable values) changes. Or so I
thought. Consider this. The current property "Maximum Rate" could be marked
as deprecated. It's functionality might even be able to continue - caveat:
as long as 'data rate and flowfile count' is not the selected criteria.
Perhaps a bulletin would be present as long as the older property is in use
drawing attention to the deprecation. At the same time, two new properties
are added with the intent of eventually keeping them and removing the
existing "Maximum Rate". If either (or both) new property is specified, the
old property will be ignored. Of course, this will all be documented in the
processor's docs.

If this is a desirable short-term solution, I'll investigate implementing
it this way.

-Mark

On Sat, Oct 8, 2022 at 5:57 PM Joe Witt <jo...@gmail.com>> wrote:

Phil

As we approach time for some house cleaning in a nifi 2.0 world such a
model/idea will be important for sure.

Mark

Breaking old behavior would be problematic.  And I think youre right that
only sorta clunky options exist with the current one.  I was thinking you
could do something with dependent properties to make it cleaner but not
sure.

I was initially thinking ‘no way’ to a new processor for just this but as
I was replying it might be the cleanest option.  Maybe call it ThrottleData
and use the same tags as control rate.

Others might have a different take obviously.

Thanks



On Sat, Oct 8, 2022 at 2:22 PM Phil H <gi...@gmail.com>> wrote:

It might be too much to do prior to solving this problem, but for the
general case, but could NiFi benefit from a @Depricated annotation (or
similar) on PropertyDescriptor. This could be read only, requiring the
admin to change to the new property.

Alternatively, the deprecated property could be hidden if a new method
was
implemented in a processor that changed old property values to new ones
(in
this case converting 10 to “10 Mb”.

Just a thought.

On Sun, 9 Oct 2022 at 02:57, Mark Bean <ma...@gmail.com>> wrote:

I am working on NIFI-10243 [1]. The goal is to allow ControlRate to
throttle based on both data rate and FlowFile count. If either rate is
exceeded, throttling occurs.

Currently, throttling occurs in only one mode. Therefore, a single
property, Maximum Rate, is overloaded to accept either a size limit or
a
count limit. Changing how this property is used could become a breaking
change. However, keeping backward compatibility makes the property
usage
less logical.

It makes good sense to have two properties, one for data rate, e.g. "1
MB",
and one for count rate, e.g. "10". Unfortunately, this implementation
would
break current instantiations of the processor since an integer value in
the
existing attribute would not be accepted any longer.

It is possible to keep the functionality the same and introduce a new
"Maximum Count Rate" property which is to be used _only_ when the Rate
Control Criteria is the new 'data rate and flowfile count'. I don't
feel
this makes the processor property usage easy to understand though. The
flowfile count rate would be provided in the existing Maximum Rate
property
if the criteria is 'flowfile count', but it would have to be specified
in a
new property if the criteria is 'data rate and flowfile count'. This is
inconsistent and confusing.

Perhaps, "Maximum Rate" property could be overloaded to accept a byte
value, an integer value or a comma-separated list of both, depending on
the
selected rate control criteria. It's clunky, but could work.

Another alternative to avoid a breaking change is to introduce a new
processor, ExtendedControlRate, which allows for the new Rate Control
Criteria and redefined rate properties.

So, the choices are:
1) Breaking change aligning properties with their purpose in an easy to
understand manner
2) Backward compatible where flowfile count is specified in different
properties depending on rate control criteria
3) Backward compatible where Maximum Rate accepts a string, an integer
or
both (comma-separated)
4) Introduce a new processor

I think option 1 makes the most sense, but I'm concerned about the
breaking
nature of the approach. Looking for suggestions on how to proceed.

[1] https://issues.apache.org/jira/browse/NIFI-10243





Re: dealing with a breaking change

Posted by Mark Bean <ma...@gmail.com>.
There was a reason I left "new processor" as the last option. In my
opinion, it's the least desirable - but honestly, it's the easiest. Still...

Deprecating the property isn't really appropriate here because the property
remains. It's just that its behavior (allowable values) changes. Or so I
thought. Consider this. The current property "Maximum Rate" could be marked
as deprecated. It's functionality might even be able to continue - caveat:
as long as 'data rate and flowfile count' is not the selected criteria.
Perhaps a bulletin would be present as long as the older property is in use
drawing attention to the deprecation. At the same time, two new properties
are added with the intent of eventually keeping them and removing the
existing "Maximum Rate". If either (or both) new property is specified, the
old property will be ignored. Of course, this will all be documented in the
processor's docs.

If this is a desirable short-term solution, I'll investigate implementing
it this way.

-Mark

On Sat, Oct 8, 2022 at 5:57 PM Joe Witt <jo...@gmail.com> wrote:

> Phil
>
> As we approach time for some house cleaning in a nifi 2.0 world such a
> model/idea will be important for sure.
>
> Mark
>
> Breaking old behavior would be problematic.  And I think youre right that
> only sorta clunky options exist with the current one.  I was thinking you
> could do something with dependent properties to make it cleaner but not
> sure.
>
>  I was initially thinking ‘no way’ to a new processor for just this but as
> I was replying it might be the cleanest option.  Maybe call it ThrottleData
> and use the same tags as control rate.
>
> Others might have a different take obviously.
>
> Thanks
>
>
>
> On Sat, Oct 8, 2022 at 2:22 PM Phil H <gi...@gmail.com> wrote:
>
> > It might be too much to do prior to solving this problem, but for the
> > general case, but could NiFi benefit from a @Depricated annotation (or
> > similar) on PropertyDescriptor. This could be read only, requiring the
> > admin to change to the new property.
> >
> > Alternatively, the deprecated property could be hidden if a new method
> was
> > implemented in a processor that changed old property values to new ones
> (in
> > this case converting 10 to “10 Mb”.
> >
> > Just a thought.
> >
> > On Sun, 9 Oct 2022 at 02:57, Mark Bean <ma...@gmail.com> wrote:
> >
> > > I am working on NIFI-10243 [1]. The goal is to allow ControlRate to
> > > throttle based on both data rate and FlowFile count. If either rate is
> > > exceeded, throttling occurs.
> > >
> > > Currently, throttling occurs in only one mode. Therefore, a single
> > > property, Maximum Rate, is overloaded to accept either a size limit or
> a
> > > count limit. Changing how this property is used could become a breaking
> > > change. However, keeping backward compatibility makes the property
> usage
> > > less logical.
> > >
> > > It makes good sense to have two properties, one for data rate, e.g. "1
> > MB",
> > > and one for count rate, e.g. "10". Unfortunately, this implementation
> > would
> > > break current instantiations of the processor since an integer value in
> > the
> > > existing attribute would not be accepted any longer.
> > >
> > > It is possible to keep the functionality the same and introduce a new
> > > "Maximum Count Rate" property which is to be used _only_ when the Rate
> > > Control Criteria is the new 'data rate and flowfile count'. I don't
> feel
> > > this makes the processor property usage easy to understand though. The
> > > flowfile count rate would be provided in the existing Maximum Rate
> > property
> > > if the criteria is 'flowfile count', but it would have to be specified
> > in a
> > > new property if the criteria is 'data rate and flowfile count'. This is
> > > inconsistent and confusing.
> > >
> > > Perhaps, "Maximum Rate" property could be overloaded to accept a byte
> > > value, an integer value or a comma-separated list of both, depending on
> > the
> > > selected rate control criteria. It's clunky, but could work.
> > >
> > > Another alternative to avoid a breaking change is to introduce a new
> > > processor, ExtendedControlRate, which allows for the new Rate Control
> > > Criteria and redefined rate properties.
> > >
> > > So, the choices are:
> > > 1) Breaking change aligning properties with their purpose in an easy to
> > > understand manner
> > > 2) Backward compatible where flowfile count is specified in different
> > > properties depending on rate control criteria
> > > 3) Backward compatible where Maximum Rate accepts a string, an integer
> or
> > > both (comma-separated)
> > > 4) Introduce a new processor
> > >
> > > I think option 1 makes the most sense, but I'm concerned about the
> > breaking
> > > nature of the approach. Looking for suggestions on how to proceed.
> > >
> > > [1] https://issues.apache.org/jira/browse/NIFI-10243
> > >
> >
>

Re: dealing with a breaking change

Posted by Joe Witt <jo...@gmail.com>.
Phil

As we approach time for some house cleaning in a nifi 2.0 world such a
model/idea will be important for sure.

Mark

Breaking old behavior would be problematic.  And I think youre right that
only sorta clunky options exist with the current one.  I was thinking you
could do something with dependent properties to make it cleaner but not
sure.

 I was initially thinking ‘no way’ to a new processor for just this but as
I was replying it might be the cleanest option.  Maybe call it ThrottleData
and use the same tags as control rate.

Others might have a different take obviously.

Thanks



On Sat, Oct 8, 2022 at 2:22 PM Phil H <gi...@gmail.com> wrote:

> It might be too much to do prior to solving this problem, but for the
> general case, but could NiFi benefit from a @Depricated annotation (or
> similar) on PropertyDescriptor. This could be read only, requiring the
> admin to change to the new property.
>
> Alternatively, the deprecated property could be hidden if a new method was
> implemented in a processor that changed old property values to new ones (in
> this case converting 10 to “10 Mb”.
>
> Just a thought.
>
> On Sun, 9 Oct 2022 at 02:57, Mark Bean <ma...@gmail.com> wrote:
>
> > I am working on NIFI-10243 [1]. The goal is to allow ControlRate to
> > throttle based on both data rate and FlowFile count. If either rate is
> > exceeded, throttling occurs.
> >
> > Currently, throttling occurs in only one mode. Therefore, a single
> > property, Maximum Rate, is overloaded to accept either a size limit or a
> > count limit. Changing how this property is used could become a breaking
> > change. However, keeping backward compatibility makes the property usage
> > less logical.
> >
> > It makes good sense to have two properties, one for data rate, e.g. "1
> MB",
> > and one for count rate, e.g. "10". Unfortunately, this implementation
> would
> > break current instantiations of the processor since an integer value in
> the
> > existing attribute would not be accepted any longer.
> >
> > It is possible to keep the functionality the same and introduce a new
> > "Maximum Count Rate" property which is to be used _only_ when the Rate
> > Control Criteria is the new 'data rate and flowfile count'. I don't feel
> > this makes the processor property usage easy to understand though. The
> > flowfile count rate would be provided in the existing Maximum Rate
> property
> > if the criteria is 'flowfile count', but it would have to be specified
> in a
> > new property if the criteria is 'data rate and flowfile count'. This is
> > inconsistent and confusing.
> >
> > Perhaps, "Maximum Rate" property could be overloaded to accept a byte
> > value, an integer value or a comma-separated list of both, depending on
> the
> > selected rate control criteria. It's clunky, but could work.
> >
> > Another alternative to avoid a breaking change is to introduce a new
> > processor, ExtendedControlRate, which allows for the new Rate Control
> > Criteria and redefined rate properties.
> >
> > So, the choices are:
> > 1) Breaking change aligning properties with their purpose in an easy to
> > understand manner
> > 2) Backward compatible where flowfile count is specified in different
> > properties depending on rate control criteria
> > 3) Backward compatible where Maximum Rate accepts a string, an integer or
> > both (comma-separated)
> > 4) Introduce a new processor
> >
> > I think option 1 makes the most sense, but I'm concerned about the
> breaking
> > nature of the approach. Looking for suggestions on how to proceed.
> >
> > [1] https://issues.apache.org/jira/browse/NIFI-10243
> >
>

Re: dealing with a breaking change

Posted by Phil H <gi...@gmail.com>.
It might be too much to do prior to solving this problem, but for the
general case, but could NiFi benefit from a @Depricated annotation (or
similar) on PropertyDescriptor. This could be read only, requiring the
admin to change to the new property.

Alternatively, the deprecated property could be hidden if a new method was
implemented in a processor that changed old property values to new ones (in
this case converting 10 to “10 Mb”.

Just a thought.

On Sun, 9 Oct 2022 at 02:57, Mark Bean <ma...@gmail.com> wrote:

> I am working on NIFI-10243 [1]. The goal is to allow ControlRate to
> throttle based on both data rate and FlowFile count. If either rate is
> exceeded, throttling occurs.
>
> Currently, throttling occurs in only one mode. Therefore, a single
> property, Maximum Rate, is overloaded to accept either a size limit or a
> count limit. Changing how this property is used could become a breaking
> change. However, keeping backward compatibility makes the property usage
> less logical.
>
> It makes good sense to have two properties, one for data rate, e.g. "1 MB",
> and one for count rate, e.g. "10". Unfortunately, this implementation would
> break current instantiations of the processor since an integer value in the
> existing attribute would not be accepted any longer.
>
> It is possible to keep the functionality the same and introduce a new
> "Maximum Count Rate" property which is to be used _only_ when the Rate
> Control Criteria is the new 'data rate and flowfile count'. I don't feel
> this makes the processor property usage easy to understand though. The
> flowfile count rate would be provided in the existing Maximum Rate property
> if the criteria is 'flowfile count', but it would have to be specified in a
> new property if the criteria is 'data rate and flowfile count'. This is
> inconsistent and confusing.
>
> Perhaps, "Maximum Rate" property could be overloaded to accept a byte
> value, an integer value or a comma-separated list of both, depending on the
> selected rate control criteria. It's clunky, but could work.
>
> Another alternative to avoid a breaking change is to introduce a new
> processor, ExtendedControlRate, which allows for the new Rate Control
> Criteria and redefined rate properties.
>
> So, the choices are:
> 1) Breaking change aligning properties with their purpose in an easy to
> understand manner
> 2) Backward compatible where flowfile count is specified in different
> properties depending on rate control criteria
> 3) Backward compatible where Maximum Rate accepts a string, an integer or
> both (comma-separated)
> 4) Introduce a new processor
>
> I think option 1 makes the most sense, but I'm concerned about the breaking
> nature of the approach. Looking for suggestions on how to proceed.
>
> [1] https://issues.apache.org/jira/browse/NIFI-10243
>