You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@nifi.apache.org by ijokarumawak <gi...@git.apache.org> on 2017/02/01 03:05:17 UTC

[GitHub] nifi pull request #1460: NIFI-3373: Add nifi.flow.configuration.archive.max....

GitHub user ijokarumawak opened a pull request:

    https://github.com/apache/nifi/pull/1460

    NIFI-3373: Add nifi.flow.configuration.archive.max.count

    - Add 'nifi.flow.configuration.archive.max.count' in nifi.properties
    - Change default archive limit so that it uses archive max time(30 days)
      and storage (500MB) if no limitation is specified
    - Simplified logic to delete old archives
    
    Thank you for submitting a contribution to Apache NiFi.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
         in the commit message?
    
    - [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [x] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
    - [x] Have you written or updated unit tests to verify your changes?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
    - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
    - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
    - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
    
    ### For documentation related changes:
    - [x] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


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

    $ git pull https://github.com/ijokarumawak/nifi nifi-3373

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

    https://github.com/apache/nifi/pull/1460.patch

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

    This closes #1460
    
----
commit 05dce3f1d3081b737a2a3112795156d5cbd640a2
Author: Koji Kawamura <ij...@apache.org>
Date:   2017-01-24T00:23:01Z

    NIFI-3373: Add nifi.flow.configuration.archive.max.count
    
    - Add 'nifi.flow.configuration.archive.max.count' in nifi.properties
    - Change default archive limit so that it uses archive max time(30 days)
      and storage (500MB) if no limitation is specified
    - Simplified logic to delete old archives

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1460: NIFI-3373: Add nifi.flow.configuration.archive.max....

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/1460


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1460: NIFI-3373: Add nifi.flow.configuration.archive.max....

Posted by joewitt <gi...@git.apache.org>.

Github user joewitt commented on a diff in the pull request:

https://github.com/apache/nifi/pull/1460#discussion_r101202901

--- Diff: nifi-docs/src/main/asciidoc/administration-guide.adoc ---
@@ -1934,9 +1934,10 @@ The first section of the _nifi.properties_ file is for the Core Properties. Thes
|nifi.version|The version number of the current release. If upgrading but reusing this file, be sure to update this value.
|nifi.flow.configuration.file*|The location of the flow configuration file (i.e., the file that contains what is currently displayed on the NiFi graph). The default value is ./conf/flow.xml.gz.
|nifi.flow.configuration.archive.enabled*|Specifies whether NiFi creates a backup copy of the flow automatically when the flow is updated. The default value is _true_.
-|nifi.flow.configuration.archive.dir*|The location of the archive directory where backup copies of the flow.xml are saved. The default value is ./conf/archive. NiFi removes old archive files to limit disk usage based on file lifespan and total size, as specified with max.time and max.storage properties below. However, this cleanup mechanism takes into account only automatically created archived flow.xml files. That is, if there are other files or directories in this archive directory, NiFi will ignore them. Automatically created archives have filename with ISO 8601 format timestamp prefix followed by '_<original-filename>'. That is <year><month><day>T<hour><minute><second>+<timezone offset>_<original filename>. For example, `20160706T160719+0900_flow.xml.gz`. NiFi checks filenames when it cleans archive directory. If you would like to keep a particular archive in this directory without worrying about NiFi deleting it, you can do so by copying it with a different filename pattern
.
-|nifi.flow.configuration.archive.max.time*|The lifespan of archived flow.xml files. NiFi will delete expired archive files when it updates flow.xml. Expiration is determined based on current system time and the last modified timestamp of an archived flow.xml. The default value is 30 days.
-|nifi.flow.configuration.archive.max.storage*|The total data size allowed for the archived flow.xml files. NiFi will delete the oldest archive files until the total archived file size becomes less than this configuration value. The default value is 500 MB.
+|nifi.flow.configuration.archive.dir*|The location of the archive directory where backup copies of the flow.xml are saved. The default value is ./conf/archive. NiFi removes old archive files to limit disk usage based on file lifespan total size, and number of files, as specified with max.time, max.storage and max.count properties described below. This cleanup mechanism takes into account only automatically created archived flow.xml files. If there are other files or directories in this archive directory, NiFi will ignore them. Automatically created archives have filename with ISO 8601 format timestamp prefix followed by '_<original-filename>'. That is <year><month><day>T<hour><minute><second>+<timezone offset>_<original filename>. For example, `20160706T160719+0900_flow.xml.gz`. NiFi checks filenames when it cleans archive directory. If you would like to keep a particular archive in this directory without worrying about NiFi deleting it, you can do so by copying it with a differ
ent filename pattern.
--- End diff --

if you do agree we should provide sensible defaults we should add/indicate them in here.

---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1460: NIFI-3373: Add nifi.flow.configuration.archive.max....

Posted by joewitt <gi...@git.apache.org>.

Github user joewitt commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1460#discussion_r101202873
  
    --- Diff: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java ---
    @@ -972,11 +973,15 @@ public Integer getFlowElectionMaxCandidates() {
         }
     
         public String getFlowConfigurationArchiveMaxTime() {
    -        return getProperty(FLOW_CONFIGURATION_ARCHIVE_MAX_TIME, DEFAULT_FLOW_CONFIGURATION_ARCHIVE_MAX_TIME);
    +        return getProperty(FLOW_CONFIGURATION_ARCHIVE_MAX_TIME, null);
    --- End diff --
    
    @ijokarumawak i think for each of these max values we should have some sensible default.  For time perhaps 14 days.  For storage perhaps 1GB.  For count 50.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #1460: NIFI-3373: Add nifi.flow.configuration.archive.max.count

Posted by joewitt <gi...@git.apache.org>.

Github user joewitt commented on the issue:

    https://github.com/apache/nifi/pull/1460
  
    @ijokarumawak thanks for explaining and yes I'm totally on track now.  thanks.  Can you please squash and merge.  +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1460: NIFI-3373: Add nifi.flow.configuration.archive.max....

Posted by ijokarumawak <gi...@git.apache.org>.

Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1460#discussion_r101427041
  
    --- Diff: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java ---
    @@ -972,11 +973,15 @@ public Integer getFlowElectionMaxCandidates() {
         }
     
         public String getFlowConfigurationArchiveMaxTime() {
    -        return getProperty(FLOW_CONFIGURATION_ARCHIVE_MAX_TIME, DEFAULT_FLOW_CONFIGURATION_ARCHIVE_MAX_TIME);
    +        return getProperty(FLOW_CONFIGURATION_ARCHIVE_MAX_TIME, null);
    --- End diff --
    
    @joewitt It used to have defaults here, for time 30 days, for storage 500MB. These defaults are still effective if none of those are specified in nifi.properties. I just moved the code applying default values to here:
    
    ```java
    // FlowConfigurationArchiveManager
    public FlowConfigurationArchiveManager(final Path flowConfigFile, final NiFiProperties properties) {
    (...)
            if (maxCount == null && StringUtils.isBlank(maxTime) && StringUtils.isBlank(maxStorage)) {
                // None of limitation is specified, fall back to the default configuration;
                maxTime = NiFiProperties.DEFAULT_FLOW_CONFIGURATION_ARCHIVE_MAX_TIME;
                maxStorage = NiFiProperties.DEFAULT_FLOW_CONFIGURATION_ARCHIVE_MAX_STORAGE;
                logger.info("None of archive max limitation is specified, fall back to the default configuration, maxTime={}, maxStorage={}",
                        maxTime, maxStorage);
            }
    ```
    https://github.com/apache/nifi/pull/1460/files#diff-00c47dfbe82807a330d4f2f9432bb54dR70
    
    The purpose of this is to let user only specify what they would like to use to limit the archiving. Some user want to limit by only archive file count and don't want to limit by size ... etc.
    To avoid keep archiving without any limit, the above code uses default time and storage size when no limit is configured.
    Does this sound reasonable?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1460: NIFI-3373: Add nifi.flow.configuration.archive.max....

Posted by ijokarumawak <gi...@git.apache.org>.

Github user ijokarumawak commented on a diff in the pull request:

https://github.com/apache/nifi/pull/1460#discussion_r101433687

Updated admin doc to describe how limitation settings work if no configuration is set for those.

[GitHub] nifi issue #1460: NIFI-3373: Add nifi.flow.configuration.archive.max.count

Posted by ijokarumawak <gi...@git.apache.org>.

Github user ijokarumawak commented on the issue:

    https://github.com/apache/nifi/pull/1460
  
    Following Joe's +1, squashed and merged it into master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---