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 
<mark.o.b...@gmail.com<mailto:mark.o.b...@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 
<joe.w...@gmail.com<mailto:joe.w...@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 
<gippyp...@gmail.com<mailto:gippyp...@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 
<mark.o.b...@gmail.com<mailto:mark.o.b...@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




Reply via email to