You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/05/13 22:24:53 UTC
[GitHub] [activemq-artemis] clebertsuconic opened a new pull request #3580: ARTEMIS-3297 Historic Journal Backup
clebertsuconic opened a new pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632851224
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
@michaelandrepearce / @gtully actually.. I take that back...
I will do days and hours...
I don't have time to test all the options now.. having more code means I would have to test it...
If we ever cross a use case of a user needing more granularity we can add it later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632892149
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -1858,6 +1988,8 @@ public synchronized void compact() throws Exception {
journalLock.writeLock().unlock();
}
+ processBackup();
Review comment:
processBackup() will copy the BKP files to the retention folder.
Before I do the computation I need any BKP files cleared so they can be treated. So, the backup is before compacting, hence I'm calling processBackup before compacting here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633771858
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
Review comment:
I will allow unlimited.. and log a warn if no value is provided.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894221
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -2245,6 +2379,155 @@ private synchronized JournalLoadInformation load(final LoaderCallback loadManage
}
}
+
+ @Override
+ public void processBackupCleanup() {
+ if (journalRetentionFolder != null && (journalRetentionMaxFiles > 0 || journalRetentionPeriod > 0)) {
+
+ FilenameFilter fnf = new FilenameFilter() {
+ @Override
+ public boolean accept(final File file, final String name) {
+ return name != null && name.endsWith("." + filesRepository.getFileExtension());
+ }
+ };
+
+
+ if (journalRetentionPeriod > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ GregorianCalendar calendar = this.calendarThreadLocal.get();
+ calendar.setTimeInMillis(System.currentTimeMillis() - journalRetentionUnit.toMillis(journalRetentionPeriod));
+ long timeCutOf = calendar.getTimeInMillis();
+
+ for (String fileName : fileNames) {
+ long timeOnFile = getDatePortionMillis(fileName);
+ if (timeOnFile < timeCutOf) {
+ logger.debug("File " + fileName + " is too old and should go");
+ File fileToRemove = new File(journalRetentionFolder, fileName);
+ if (!fileToRemove.delete()) {
+ logger.debug("Could not remove " + fileToRemove);
+ }
+ }
+ }
+ }
+
+ if (journalRetentionMaxFiles > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ if (fileNames.length > journalRetentionMaxFiles) {
+ int toRemove = fileNames.length - journalRetentionMaxFiles;
+
+ for (String file : fileNames) {
+ logger.debug("Removing " + file);
+ File fileToRemove = new File(journalRetentionFolder, file);
+ fileToRemove.delete();
+ toRemove--;
+ if (toRemove <= 0) {
+ break;
+ }
+ }
+ }
+ }
+
+
+ }
+ }
+
+ @Override
+ public void processBackup() {
+ if (journalRetentionFolder != null) {
+ ArrayList<JournalFile> filesToMove;
+ synchronized (historyPendingFiles) {
+ filesToMove = new ArrayList<>(historyPendingFiles.size());
+ filesToMove.addAll(historyPendingFiles);
+ historyPendingFiles.clear();
+ }
+
+
+ Calendar calendar = calendarThreadLocal.get();
Review comment:
because of the change i was thinking there was maybe more to it. I guess just consistency so in future people not double thinking like i just did why the difference
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632888592
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
Review comment:
Also why seperate the unit and the period why not as a single attribute string like for storage where its value and scaler
E.g. 2040594944ms 360s 4h 7d and if no unit default to ms
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632787098
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
Hmmm.. this feature could also be used for Disaster and Recovery options.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633902141
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
dont forget to add <xsd:attributeGroup ref="xml:specialAttrs"/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633860816
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
I dont follow. Im sure if its enabled by declaring the xml were ensuring that directory is NOT optional.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632889528
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,45 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String directory = getAttributeValue(node, "directory");
+ long storageLimit = ByteUtil.convertTextBytes(getAttributeValue(node, "storage-limit").trim());
+ int period = getAttributeInteger(node, "period", 0, Validators.GT_ZERO);
+ String unitStr = getAttributeValue(node, "unit");
+
+ TimeUnit unit;
+
+ switch (unitStr) {
+ case "DAYS":
+ unit = TimeUnit.DAYS;
+ break;
+ case "HOURS":
+ unit = TimeUnit.HOURS;
+ break;
+ case "MINUTES":
+ unit = TimeUnit.MINUTES;
+ break;
+ case "SECONDS":
+ unit = TimeUnit.SECONDS;
+ break;
+ default:
+ unit = TimeUnit.DAYS;
+ }
+
+ config.setJournalRetentionDirectory(directory);
+ config.setJournalRetentionTimeUnit(unit);
Review comment:
So that two args throughout code base dont have to deal with time conversion and also in more critical code it doesnt have to be converted later why not convert to ms at point of config load
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632893268
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Yes but other dirs the element value is the directory name... not saying to move attributes for the retention parts. Just saying shouldnt the directory attribute be the element value as like other data dirs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632578418
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
period does not work for files: maybe:
`<journal-retention limit-type="files|days|hours|seconds" limit="number > 0" folder="data/history"/>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632508629
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
@gtully I thought about using max-files for such cases.. Humans are usually slow to react... ..
perhaps we could have this
```
<journal-retention period-type="days|files|hours" period="Number" folder="data/history"/>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632592880
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
I would suggest a timebased config item and a files/size based config item
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632855325
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
I was going to take it back, but I actually wrote a test to validate the options on the XML.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632592880
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
I would suggest a timebased config item and a separate files/size based config item to allow this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633899129
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
```
<xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
the directory to store journal-retention message in and rention configuraion.
</xsd:documentation>
</xsd:annotation>
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="unit" use="optional">
<xsd:annotation>
<xsd:documentation>
This configures the period type to use on limit. By default it is DAYS.
</xsd:documentation>
</xsd:annotation>
<xsd:simpleType>
<xsd:restriction base="xsd:string">
<xsd:enumeration value="DAYS"/>
<xsd:enumeration value="HOURS"/>
<xsd:enumeration value="SECONDS"/>
</xsd:restriction>
</xsd:simpleType>
</xsd:attribute>
<xsd:attribute name="period" type="xsd:integer" use="optional">
<xsd:annotation>
<xsd:documentation>
The amount of time used to keep files.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="storage-limit" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation>
Size (in bytes) before we starting removing files from the retention area.
this is an extra protection on top of the period.
Notice we first remove files based on period and if you're using more storage then you
configured we start removing older files.
By default this is unlimited (not filled).
Supports byte notation like "K", "Mb", "GB", etc.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633923879
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,64 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention-directory");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String directory = node.getTextContent().trim();
+
+ String storageLimitStr = getAttributeValue(node, "storage-limit");
+ long storageLimit;
+
+ if (storageLimitStr == null) {
+ storageLimit = -1;
+ } else {
+ storageLimit = ByteUtil.convertTextBytes(storageLimitStr.trim());
+ }
+ int period = getAttributeInteger(node, "period", -1, Validators.GT_ZERO);
+ String unitStr = getAttributeValue(node, "unit");
+
+ if (unitStr == null) {
+ unitStr = "DAYS";
+ }
+
+ TimeUnit unit;
+
+ switch (unitStr) {
Review comment:
Nice.. idea.. done!
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,64 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention-directory");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String directory = node.getTextContent().trim();
+
+ String storageLimitStr = getAttributeValue(node, "storage-limit");
+ long storageLimit;
+
+ if (storageLimitStr == null) {
+ storageLimit = -1;
+ } else {
+ storageLimit = ByteUtil.convertTextBytes(storageLimitStr.trim());
+ }
+ int period = getAttributeInteger(node, "period", -1, Validators.GT_ZERO);
+ String unitStr = getAttributeValue(node, "unit");
+
+ if (unitStr == null) {
+ unitStr = "DAYS";
+ }
+
+ TimeUnit unit;
+
+ switch (unitStr) {
Review comment:
Nice idea.. done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632891635
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
Review comment:
it wouldn't be as clear. the XSD is pretty standard.. if you edit the XML with an editor you get auto-completion and self documented. As if you use the schema you use on bytes it would be free form... more error prone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632350082
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java
##########
@@ -668,6 +668,25 @@ Configuration addDiscoveryGroupConfiguration(String key,
*/
Configuration setJournalDirectory(String dir);
+ String getJournalHistory();
+
+ File getJournalHistoryLocation();
+
+ /**
+ * Sets the file system directory used to store historical backup journal.
+ */
+ Configuration setJournalHistory(String dir);
+
+ int getJournalHistoryMaxDays();
+
+ Configuration setJournalHistoryMaxDays(int days);
Review comment:
This is a neat feature. I think I would call it something else from a configuration perspective.
journalRetention or just retention
configured with a:
int retentionLimit > 0
enum retentionUnit (seconds/days/numFiles)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632753627
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
> files is good if you care about you max disk usage. But I think for time, hours is too coarse. and seconds covers everything up to hours nicely.
Initial idea was for it to be coarse as it would be days/weeks retention, intent is to time machine back after say an incident that gets found out after the fact and work out what could have happened. I’m not sure how useful seconds here would be tbh. That said if the value is a long having it just as simple ms like retention is in kafka just simplifies config, everyone is able to convert days/weeks to ms and the we have a simple config, not having to deal with variants
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633860816
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
I dont follow. Im sure if its enabled by declaring the xml were ensuring that directory is NOT optional, as such we shouldnt be defaulting the value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632888143
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
Review comment:
maybuscan use xsd assert to handle more complex checks that at least size or time is set and if time that period is set
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633902925
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="storage-limit" type="xsd:string" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ Size (in bytes) before we starting removing files from the retention area.
+ this is an extra protection on top of the period.
+ Notice we first remove files based on period and if you're using more storage then you configured we start removing older files.
+ By default this is unlimited (not filled).
+ Supports byte notation like "K", "Mb", "GB", etc.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ </xsd:complexType>
+ </xsd:element>
+
+
+
+ <xsd:element name="journal-history-directory" type="xsd:string" default="" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ The directory to store historical information about the data on the broker.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:element>
+
+ <xsd:element name="journal-history-max-days" type="xsd:integer" default="-1" maxOccurs="1" minOccurs="0">
Review comment:
is this dead now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633916801
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,64 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention-directory");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String directory = node.getTextContent().trim();
+
+ String storageLimitStr = getAttributeValue(node, "storage-limit");
+ long storageLimit;
+
+ if (storageLimitStr == null) {
+ storageLimit = -1;
+ } else {
+ storageLimit = ByteUtil.convertTextBytes(storageLimitStr.trim());
+ }
+ int period = getAttributeInteger(node, "period", -1, Validators.GT_ZERO);
+ String unitStr = getAttributeValue(node, "unit");
+
+ if (unitStr == null) {
+ unitStr = "DAYS";
+ }
+
+ TimeUnit unit;
+
+ switch (unitStr) {
Review comment:
Why not use TimeUnit.valueOf its already an enum?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] asfgit closed pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632887693
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
Review comment:
Same comment as below what if someone wants to limit just on size
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894284
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -108,6 +115,7 @@
*
* */
public static final double UPDATE_FACTOR;
+ public static final String BKP = ".bkp";
Review comment:
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632887633
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
Review comment:
What if someone wants to limit just by size?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633902141
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
dont forget to add <xsd:attributeGroup ref="xml:specialAttrs"/> to support splitting the broker.xml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632508629
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
@gtully I thought about using max-files for such cases.. Humans are usually slow to react... ..
perhaps we could have this
<journal-retention period-type="days|files|hours" period="Number" folder="data/history"/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632889877
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -2245,6 +2379,155 @@ private synchronized JournalLoadInformation load(final LoaderCallback loadManage
}
}
+
+ @Override
+ public void processBackupCleanup() {
+ if (journalRetentionFolder != null && (journalRetentionMaxFiles > 0 || journalRetentionPeriod > 0)) {
+
+ FilenameFilter fnf = new FilenameFilter() {
+ @Override
+ public boolean accept(final File file, final String name) {
+ return name != null && name.endsWith("." + filesRepository.getFileExtension());
+ }
+ };
+
+
+ if (journalRetentionPeriod > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ GregorianCalendar calendar = this.calendarThreadLocal.get();
+ calendar.setTimeInMillis(System.currentTimeMillis() - journalRetentionUnit.toMillis(journalRetentionPeriod));
Review comment:
Why doing the conversion to ms every time, why not convert to ms just once all the way up at point of reading config
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632893707
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
you mean this?
```
<journal-retention unit="DAYS" period="7" storage-limit="10G">data/retention</journal-retention>
```
I would have to call it journal-retention-directory.. it wouldn't look as nice IMO. I would probably have to split it into separate elements... I would rather keep the way it is now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632890217
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -1858,6 +1988,8 @@ public synchronized void compact() throws Exception {
journalLock.writeLock().unlock();
}
+ processBackup();
Review comment:
I thought during our discussions we want the backup before compaction so its literally as raw as it gets.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632891935
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/dataformat/JournalAddRecord.java
##########
@@ -30,32 +30,37 @@
protected final byte recordType;
- protected final boolean add;
+ protected final byte journalType;
Review comment:
It is... Compatibility tests would be failing if it wasn't.
addBytes will write 1 for true, 0 for false.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633913845
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
<xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
the directory to store e journal-retention will take place.
</xsd:documentation>
</xsd:annotation>
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="unit" use="optional" type="timeUnit">
<xsd:annotation>
<xsd:documentation>
This configures the period type to use on limit. By default it is DAYS.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="period" type="xsd:integer" use="optional">
<xsd:annotation>
<xsd:documentation>
The amount of time used to keep files.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="storage-limit" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation>
Size (in bytes) before we starting removing files from the retention area.
this is an extra protection on top of the period.
Notice we first remove files based on period and if you're using more storage then you
configured we start removing older files.
By default this is unlimited (not filled).
Supports byte notation like "K", "Mb", "GB", etc.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>
And also define timeUnit so its re-usable going forwards, after all its java TimeUnit that we use to convert.
<xsd:simpleType name="timeUnit">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="DAYS"/>
<xsd:enumeration value="HOURS"/>
<xsd:enumeration value="MINUTES"/>
<xsd:enumeration value="SECONDS"/>
</xsd:restriction>
</xsd:simpleType>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632565882
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
files is good if you care about you max disk usage. But I think for time, hours is too coarse. and seconds covers everything up to hours nicely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632893752
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,45 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String directory = getAttributeValue(node, "directory");
+ long storageLimit = ByteUtil.convertTextBytes(getAttributeValue(node, "storage-limit").trim());
+ int period = getAttributeInteger(node, "period", 0, Validators.GT_ZERO);
+ String unitStr = getAttributeValue(node, "unit");
+
+ TimeUnit unit;
+
+ switch (unitStr) {
+ case "DAYS":
+ unit = TimeUnit.DAYS;
+ break;
+ case "HOURS":
+ unit = TimeUnit.HOURS;
+ break;
+ case "MINUTES":
+ unit = TimeUnit.MINUTES;
+ break;
+ case "SECONDS":
+ unit = TimeUnit.SECONDS;
+ break;
+ default:
+ unit = TimeUnit.DAYS;
+ }
+
+ config.setJournalRetentionDirectory(directory);
+ config.setJournalRetentionTimeUnit(unit);
Review comment:
I will think about this between tomorrow (Saturday) and Monday.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-842768044
This should be ready to be merged now.. I'm running one final round of tests.. and if everything goes well I will merge it tomorrow... further changes can be followed as with any module, but I think this is in good shape now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633895236
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
I believe sample type in the xsd is transportType
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633899129
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
```
<xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
the directory to store journal-retention message in and rention configuraion.
</xsd:documentation>
</xsd:annotation>
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="unit" use="optional">
<xsd:annotation>
<xsd:documentation>
This configures the period type to use on limit. By default it is DAYS.
</xsd:documentation>
</xsd:annotation>
<xsd:simpleType>
<xsd:restriction base="xsd:string">
<xsd:enumeration value="DAYS"/>
<xsd:enumeration value="HOURS"/>
<xsd:enumeration value="SECONDS"/>
</xsd:restriction>
</xsd:simpleType>
</xsd:attribute>
<xsd:attribute name="period" type="xsd:integer" use="optional">
<xsd:annotation>
<xsd:documentation>
The amount of time used to keep files.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="storage-limit" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation>
Size (in bytes) before we starting removing files from the retention area.
this is an extra protection on top of the period.
Notice we first remove files based on period and if you're using more storage then you
configured we start removing older files.
By default this is unlimited (not filled).
Supports byte notation like "K", "Mb", "GB", etc.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attributeGroup ref="xml:specialAttrs"/>
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632753627
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
> files is good if you care about you max disk usage. But I think for time, hours is too coarse. and seconds covers everything up to hours nicely.
Initial idea was for it to be coarse as it wood be days/weeks, intent is to time machine back after say an incident that gets found out after the fact and work out what could have happened. I’m not sure how useful seconds here would be tbh.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain edited a comment on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-842705224
One more query, im trying to work out here, is the copy done whilst and journal is blocked for write, or is it being done after like with compaction and release not blocking the journal writes on the current file, but simply before we do do the release or compaction, so journal writing to next file is not blocked whilst we copy old one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633913845
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
<xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
the directory to store e journal-retention will take place.
</xsd:documentation>
</xsd:annotation>
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="unit" type="timeUnit" use="optional">
<xsd:annotation>
<xsd:documentation>
This configures the period type to use on limit. By default it is DAYS.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="period" type="xsd:integer" use="optional">
<xsd:annotation>
<xsd:documentation>
The amount of time used to keep files.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="storage-limit" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation>
Size (in bytes) before we starting removing files from the retention area.
this is an extra protection on top of the period.
Notice we first remove files based on period and if you're using more storage then you
configured we start removing older files.
By default this is unlimited (not filled).
Supports byte notation like "K", "Mb", "GB", etc.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>
And also define timeUnit so its re-usable going forwards, after all its java TimeUnit that we use to convert.
<xsd:simpleType name="timeUnit">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="DAYS"/>
<xsd:enumeration value="HOURS"/>
<xsd:enumeration value="MINUTES"/>
<xsd:enumeration value="SECONDS"/>
</xsd:restriction>
</xsd:simpleType>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633909870
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
Review comment:
In artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java seems we support also Minutes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632891984
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -2245,6 +2379,155 @@ private synchronized JournalLoadInformation load(final LoaderCallback loadManage
}
}
+
+ @Override
+ public void processBackupCleanup() {
+ if (journalRetentionFolder != null && (journalRetentionMaxFiles > 0 || journalRetentionPeriod > 0)) {
+
+ FilenameFilter fnf = new FilenameFilter() {
+ @Override
+ public boolean accept(final File file, final String name) {
+ return name != null && name.endsWith("." + filesRepository.getFileExtension());
+ }
+ };
+
+
+ if (journalRetentionPeriod > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ GregorianCalendar calendar = this.calendarThreadLocal.get();
+ calendar.setTimeInMillis(System.currentTimeMillis() - journalRetentionUnit.toMillis(journalRetentionPeriod));
Review comment:
ok.. fair enough.. I can do that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632630456
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
yep, I agree, time and space, probably need both, to limit disk usage in case the time limits is too large.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633917325
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,64 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention-directory");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String directory = node.getTextContent().trim();
+
+ String storageLimitStr = getAttributeValue(node, "storage-limit");
+ long storageLimit;
+
+ if (storageLimitStr == null) {
+ storageLimit = -1;
+ } else {
+ storageLimit = ByteUtil.convertTextBytes(storageLimitStr.trim());
+ }
+ int period = getAttributeInteger(node, "period", -1, Validators.GT_ZERO);
+ String unitStr = getAttributeValue(node, "unit");
+
+ if (unitStr == null) {
+ unitStr = "DAYS";
+ }
+
+ TimeUnit unit;
+
+ switch (unitStr) {
Review comment:
e.g. this switch could be simplified down to:
unit = TimeUnit.valueOf(unitStr)
or if you want to be case safe:, but xsd anyhow should ensure its upper by type def, so probably not needed.
unit = TimeUnit.valueOf(unitStr.toUpperCase());
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-842727677
@michaelpearce-gain it should be done after...
I'm renaming the file since the lock... but the actual copy should be done outside of any locking.
there might be some tweaking to be done into it though. but that was my intention at least.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894333
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/dataformat/JournalAddRecord.java
##########
@@ -30,32 +30,37 @@
protected final byte recordType;
- protected final boolean add;
+ protected final byte journalType;
Review comment:
Cool just checking
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632891821
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
the element is journal-retention.. you simply enable it or disable it...
I think it is very neat the way it's done now.
you just comment this from the xml, and you have it working:
```xml
<journal-retention unit="DAYS" directory="data/retention" period="7" storage-limit="10G"/>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632890373
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -1858,6 +1988,8 @@ public synchronized void compact() throws Exception {
journalLock.writeLock().unlock();
}
+ processBackup();
Review comment:
Or am i miss understanding whats going on here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-841597529
@michaelandrepearce RecoverTest as I added is dealing with Transaction, non TX, Paging, non Paging, large message, non large message, NIO, MAPPED, AIO ... all with combinations.. trying to capture all possible scenarios.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894507
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Im one for consistency, as the saying goes consistency is king. Especially as a user mindset. Like why all other dirs the element value yet here its an empty element value but its an attribute. Having things miss match in style makes it harder as you have to cross check everytime. Its easier to eyeball also when cross checking if there's consistency.
Its like codestyle/formatting having it all the same style and format just makes it easier to expect certain things and easier to read. You dont want a code base with different codestyles and formats it very quickly makes it harder to read.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632592880
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
I would suggest a timebased config item/attribute and a separate files/size based config item/attribute to allow this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-841597351
@michaelandrepearce that's happening on the JournalstorageManager::pageWrite
https://github.com/clebertsuconic/activemq-artemis/blob/backup-journal/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java#L410-L427
I'm using the storeAddEvent, using the same record type as the Add Message and addRef.
Basically if you recover a message from the "paging area" it will come back to the journal.
(at least for now).
We could use the same to write to Paging.. or simply do the route to whatever is the best at the time of recovering. But at this version this is always coming back to the journal no matter where it came from.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633786716
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
Review comment:
I will actually require one filled.. either period or storage and throw an error if not passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632767871
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
@michaelandrepearce some time ago, I talked to an Architect at my job who wanted to get diffs from the broker and send over to another site from time to time.
This kind of feature would be useful for such case maybe?
It wouldn't hurt to have it though.. it's just an additional option.
I am thinking to have a commented out xml on the broker.xml that will have 7 days, 10G as the config. So if users just uncomment that they will have the 7 days and 10G max disk.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632591760
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
What if you want to limit by files and time at same time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632890542
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -108,6 +115,7 @@
*
* */
public static final double UPDATE_FACTOR;
+ public static final String BKP = ".bkp";
Review comment:
If its going to another directory do we really need or even want a suffix on it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633886272
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
I can't make it this on the xsd:
```xml
<journal-retention-directory period="x">directory</journal-retention-directory
```
There are other complex types on the XSD for the configuration.. I don't see an issue with this TBH... And I really need to finish this task as I need to finish the release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632752395
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
``
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633913990
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
ok.. thanks.. that works
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633914807
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,52 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ the directory to store journal-retention message in and rention configuraion.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:simpleContent>
+ <xsd:extension base="xsd:string">
+ <xsd:attribute name="unit" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
Review comment:
we probably want to add MINUTES as the artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java added, seems to handle it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894507
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Im one for consistency, as the saying goes consistency is king. Especially as a user mindset. Like why all other dirs the element value yet here its an empty element value but its an attribute. Having things miss match in style makes it harder as you have to cross check everytime. Its easier to eyeball also when cross checking if there's consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632891549
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
Review comment:
I think we should require a period configured. make it large if you wish (365 days e.g)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633858007
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
It is not possible to have both attributes and default children on a xml type. I will go with what I had before... and I need to move on on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632892238
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -108,6 +115,7 @@
*
* */
public static final double UPDATE_FACTOR;
+ public static final String BKP = ".bkp";
Review comment:
This is for the current journal file.
I rename it to BKP, in case the node crashes before the copy is finished, when restarted the copy is finished and the journal name restored. This is for the pending files to be copied from the journal to the BKP area.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632889667
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/dataformat/JournalAddRecord.java
##########
@@ -30,32 +30,37 @@
protected final byte recordType;
- protected final boolean add;
+ protected final byte journalType;
Review comment:
Is this back compatible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632892000
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -2245,6 +2379,155 @@ private synchronized JournalLoadInformation load(final LoaderCallback loadManage
}
}
+
+ @Override
+ public void processBackupCleanup() {
+ if (journalRetentionFolder != null && (journalRetentionMaxFiles > 0 || journalRetentionPeriod > 0)) {
+
+ FilenameFilter fnf = new FilenameFilter() {
+ @Override
+ public boolean accept(final File file, final String name) {
+ return name != null && name.endsWith("." + filesRepository.getFileExtension());
+ }
+ };
+
+
+ if (journalRetentionPeriod > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ GregorianCalendar calendar = this.calendarThreadLocal.get();
+ calendar.setTimeInMillis(System.currentTimeMillis() - journalRetentionUnit.toMillis(journalRetentionPeriod));
+ long timeCutOf = calendar.getTimeInMillis();
+
+ for (String fileName : fileNames) {
+ long timeOnFile = getDatePortionMillis(fileName);
+ if (timeOnFile < timeCutOf) {
+ logger.debug("File " + fileName + " is too old and should go");
+ File fileToRemove = new File(journalRetentionFolder, fileName);
+ if (!fileToRemove.delete()) {
+ logger.debug("Could not remove " + fileToRemove);
+ }
+ }
+ }
+ }
+
+ if (journalRetentionMaxFiles > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ if (fileNames.length > journalRetentionMaxFiles) {
+ int toRemove = fileNames.length - journalRetentionMaxFiles;
+
+ for (String file : fileNames) {
+ logger.debug("Removing " + file);
+ File fileToRemove = new File(journalRetentionFolder, file);
+ fileToRemove.delete();
+ toRemove--;
+ if (toRemove <= 0) {
+ break;
+ }
+ }
+ }
+ }
+
+
+ }
+ }
+
+ @Override
+ public void processBackup() {
+ if (journalRetentionFolder != null) {
+ ArrayList<JournalFile> filesToMove;
+ synchronized (historyPendingFiles) {
+ filesToMove = new ArrayList<>(historyPendingFiles.size());
+ filesToMove.addAll(historyPendingFiles);
+ historyPendingFiles.clear();
+ }
+
+
+ Calendar calendar = calendarThreadLocal.get();
Review comment:
It doesn't make a difference really? the threadLocal is still a GregorianCalendar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633907161
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="storage-limit" type="xsd:string" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ Size (in bytes) before we starting removing files from the retention area.
+ this is an extra protection on top of the period.
+ Notice we first remove files based on period and if you're using more storage then you configured we start removing older files.
+ By default this is unlimited (not filled).
+ Supports byte notation like "K", "Mb", "GB", etc.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ </xsd:complexType>
+ </xsd:element>
+
+
+
+ <xsd:element name="journal-history-directory" type="xsd:string" default="" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ The directory to store historical information about the data on the broker.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:element>
+
+ <xsd:element name="journal-history-max-days" type="xsd:integer" default="-1" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ The maximum number of days to keep on the history journal folder backup.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:element>
+
+ <xsd:element name="journal-history-max-files" type="xsd:integer" default="-1" maxOccurs="1" minOccurs="0">
Review comment:
yes... max-files will be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894234
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -1858,6 +1988,8 @@ public synchronized void compact() throws Exception {
journalLock.writeLock().unlock();
}
+ processBackup();
Review comment:
Understood
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632508846
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java
##########
@@ -668,6 +668,25 @@ Configuration addDiscoveryGroupConfiguration(String key,
*/
Configuration setJournalDirectory(String dir);
+ String getJournalHistory();
+
+ File getJournalHistoryLocation();
+
+ /**
+ * Sets the file system directory used to store historical backup journal.
+ */
+ Configuration setJournalHistory(String dir);
+
+ int getJournalHistoryMaxDays();
+
+ Configuration setJournalHistoryMaxDays(int days);
Review comment:
Oooohhhh... I like the name retention!!! Really nice!!!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894507
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Im one for consistency. Especially as a user mindset. Like why all other dirs the element value yet here its an empty element value but its an attribute.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633884308
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
the other data dirs don't have attributes. I don't want to get stuck over this TBH.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633899129
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
```
<xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
the directory to store journal-retention message in and rention configuraion.
</xsd:documentation>
</xsd:annotation>
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="unit" use="optional">
<xsd:annotation>
<xsd:documentation>
This configures the period type to use on limit. By default it is DAYS.
</xsd:documentation>
</xsd:annotation>
<xsd:simpleType>
<xsd:restriction base="xsd:string">
<xsd:enumeration value="DAYS"/>
<xsd:enumeration value="HOURS"/>
<xsd:enumeration value="SECONDS"/>
</xsd:restriction>
</xsd:simpleType>
</xsd:attribute>
<xsd:attribute name="period" type="xsd:integer" use="optional">
<xsd:annotation>
<xsd:documentation>
The amount of time used to keep files.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="storage-limit" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation>
Size (in bytes) before we starting removing files from the retention area.
this is an extra protection on top of the period.
Notice we first remove files based on period and if you're using more storage then you
configured we start removing older files.
By default this is unlimited (not filled).
Supports byte notation like "K", "Mb", "GB", etc.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633902141
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
dont forget to add <xsd:attributeGroup ref="xml:specialAttrs"/> to support splitting the broker.xml, if anyone wants to put this section into its own config xml and import it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632893681
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Eg to be more consistent
`<journal-retention unit="DAYS" period="7" storage-limit="10G">data/retention</journal-retention>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633902983
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="storage-limit" type="xsd:string" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ Size (in bytes) before we starting removing files from the retention area.
+ this is an extra protection on top of the period.
+ Notice we first remove files based on period and if you're using more storage then you configured we start removing older files.
+ By default this is unlimited (not filled).
+ Supports byte notation like "K", "Mb", "GB", etc.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ </xsd:complexType>
+ </xsd:element>
+
+
+
+ <xsd:element name="journal-history-directory" type="xsd:string" default="" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ The directory to store historical information about the data on the broker.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:element>
+
+ <xsd:element name="journal-history-max-days" type="xsd:integer" default="-1" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ The maximum number of days to keep on the history journal folder backup.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:element>
+
+ <xsd:element name="journal-history-max-files" type="xsd:integer" default="-1" maxOccurs="1" minOccurs="0">
Review comment:
is this dead now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632894507
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Im one for consistency. Especially as a user mindset. Like why all other dirs the element value yet here its an empty element value but its an attribute. Having things miss match in style makes it harder as you have to cross check everytime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632893681
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Eg to be more consistent
`<journal-retention-directory unit="DAYS" period="7" storage-limit="10G">data/retention</journal-retention-directory>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633914807
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,52 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ the directory to store journal-retention message in and rention configuraion.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:simpleContent>
+ <xsd:extension base="xsd:string">
+ <xsd:attribute name="unit" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
Review comment:
we probably want to add MINUTES as the artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java added, seems to handle it.
https://github.com/apache/activemq-artemis/pull/3580/files#diff-8dc1e492ab03424ea3295ef378ad5a724f3ed60d3c62ac00c8821692e411da89R808
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632893681
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Eg
`<journal-retention unit="DAYS" period="7" storage-limit="10G">data/retention</journal-retention>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632890048
##########
File path: artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##########
@@ -2245,6 +2379,155 @@ private synchronized JournalLoadInformation load(final LoaderCallback loadManage
}
}
+
+ @Override
+ public void processBackupCleanup() {
+ if (journalRetentionFolder != null && (journalRetentionMaxFiles > 0 || journalRetentionPeriod > 0)) {
+
+ FilenameFilter fnf = new FilenameFilter() {
+ @Override
+ public boolean accept(final File file, final String name) {
+ return name != null && name.endsWith("." + filesRepository.getFileExtension());
+ }
+ };
+
+
+ if (journalRetentionPeriod > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ GregorianCalendar calendar = this.calendarThreadLocal.get();
+ calendar.setTimeInMillis(System.currentTimeMillis() - journalRetentionUnit.toMillis(journalRetentionPeriod));
+ long timeCutOf = calendar.getTimeInMillis();
+
+ for (String fileName : fileNames) {
+ long timeOnFile = getDatePortionMillis(fileName);
+ if (timeOnFile < timeCutOf) {
+ logger.debug("File " + fileName + " is too old and should go");
+ File fileToRemove = new File(journalRetentionFolder, fileName);
+ if (!fileToRemove.delete()) {
+ logger.debug("Could not remove " + fileToRemove);
+ }
+ }
+ }
+ }
+
+ if (journalRetentionMaxFiles > 0) {
+ String[] fileNames = journalRetentionFolder.list(fnf);
+ Arrays.sort(fileNames);
+
+ if (fileNames.length > journalRetentionMaxFiles) {
+ int toRemove = fileNames.length - journalRetentionMaxFiles;
+
+ for (String file : fileNames) {
+ logger.debug("Removing " + file);
+ File fileToRemove = new File(journalRetentionFolder, file);
+ fileToRemove.delete();
+ toRemove--;
+ if (toRemove <= 0) {
+ break;
+ }
+ }
+ }
+ }
+
+
+ }
+ }
+
+ @Override
+ public void processBackup() {
+ if (journalRetentionFolder != null) {
+ ArrayList<JournalFile> filesToMove;
+ synchronized (historyPendingFiles) {
+ filesToMove = new ArrayList<>(historyPendingFiles.size());
+ filesToMove.addAll(historyPendingFiles);
+ historyPendingFiles.clear();
+ }
+
+
+ Calendar calendar = calendarThreadLocal.get();
Review comment:
Why in some places using gregorian and her just plain calendar
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632888982
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Why is this an attribute but for all other data directories its the element value? Just a consistency thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632578418
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
period does not work for files: maybe:
<journal-retention limit-type="files|days|hours|seconds" limit="number > 0" folder="data/history"/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633913845
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
slightly improved, separating out the type representing TimeUnit
<xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
the directory to store e journal-retention will take place.
</xsd:documentation>
</xsd:annotation>
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="unit" type="timeUnit" use="optional">
<xsd:annotation>
<xsd:documentation>
This configures the period type to use on limit. By default it is DAYS.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="period" type="xsd:integer" use="optional">
<xsd:annotation>
<xsd:documentation>
The amount of time used to keep files.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="storage-limit" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation>
Size (in bytes) before we starting removing files from the retention area.
this is an extra protection on top of the period.
Notice we first remove files based on period and if you're using more storage then you
configured we start removing older files.
By default this is unlimited (not filled).
Supports byte notation like "K", "Mb", "GB", etc.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>
And also define timeUnit so its re-usable going forwards, after all its java TimeUnit that we use to convert.
<xsd:simpleType name="timeUnit">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="DAYS"/>
<xsd:enumeration value="HOURS"/>
<xsd:enumeration value="MINUTES"/>
<xsd:enumeration value="SECONDS"/>
</xsd:restriction>
</xsd:simpleType>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632579850
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
<journal-retention limit-type="files|days|hours|seconds" limit="number > 0" folder="data/history"/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633899129
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
```
<xsd:element name="journal-retention-directory" maxOccurs="1" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
the directory to store journal-retention message in and rention configuraion.
</xsd:documentation>
</xsd:annotation>
<xsd:complexType>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="unit" use="optional">
<xsd:annotation>
<xsd:documentation>
This configures the period type to use on limit. By default it is DAYS.
</xsd:documentation>
</xsd:annotation>
<xsd:simpleType>
<xsd:restriction base="xsd:string">
<xsd:enumeration value="DAYS"/>
<xsd:enumeration value="HOURS"/>
<xsd:enumeration value="MINUTES"/>
<xsd:enumeration value="SECONDS"/>
</xsd:restriction>
</xsd:simpleType>
</xsd:attribute>
<xsd:attribute name="period" type="xsd:integer" use="optional">
<xsd:annotation>
<xsd:documentation>
The amount of time used to keep files.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="storage-limit" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation>
Size (in bytes) before we starting removing files from the retention area.
this is an extra protection on top of the period.
Notice we first remove files based on period and if you're using more storage then you
configured we start removing older files.
By default this is unlimited (not filled).
Supports byte notation like "K", "Mb", "GB", etc.
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>
</xsd:element>```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-842705224
One more query, im trying to work out here, is the copy done whilst and journal is blocked for write, or is it being done after like with compaction and release not blocking the journal writes on the current file, but simply before we do do the release or compaction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633892984
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
Its about consistency. And the issue is once released we cant easily change it.
We actually already have such type on xsd/xml. connectors and acceptors which have a value as well as attributes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic edited a comment on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
clebertsuconic edited a comment on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-842727677
@michaelpearce-gain it should be done after...
I'm renaming the file inside the lock... but the actual copy should be done outside of any locking.
there might be some tweaking to be done into it though. but that was my intention at least.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#issuecomment-841595400
@clebertsuconic looks good left some comments / queries. On top of that just for my own benefit as i couldnt figure it out, where is it that data that would page is now still written to journal so that we get a pure journal? I see large message was dealt with but couldn't spot records that paged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633909541
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,63 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String storageLimitStr = getAttributeValue(node, "storage-limit");
+ long storageLimit;
+
+ if (storageLimitStr == null) {
+ storageLimit = -1;
+ } else {
+ storageLimit = ByteUtil.convertTextBytes(storageLimitStr.trim());
+ }
+ int period = getAttributeInteger(node, "period", -1, Validators.GT_ZERO);
+ String directory = getAttributeValue(node, "directory");
+ String unitStr = getAttributeValue(node, "unit");
+
+ if (unitStr == null) {
+ unitStr = "DAYS";
+ }
+
+ TimeUnit unit;
+
+ switch (unitStr) {
+ case "DAYS":
+ unit = TimeUnit.DAYS;
+ break;
+ case "HOURS":
+ unit = TimeUnit.HOURS;
+ break;
+ case "MINUTES":
Review comment:
Here we support minutes but schema only has hours, days, seconds, i guess we want to add minutes to the schema / xsd
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633917325
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java
##########
@@ -768,6 +772,64 @@ public void parseMainConfig(final Element e, final Configuration config) throws
}
}
+
+ private void parseJournalRetention(final Element e, final Configuration config) {
+ NodeList retention = e.getElementsByTagName("journal-retention-directory");
+
+ if (retention.getLength() != 0) {
+ Element node = (Element) retention.item(0);
+
+ String directory = node.getTextContent().trim();
+
+ String storageLimitStr = getAttributeValue(node, "storage-limit");
+ long storageLimit;
+
+ if (storageLimitStr == null) {
+ storageLimit = -1;
+ } else {
+ storageLimit = ByteUtil.convertTextBytes(storageLimitStr.trim());
+ }
+ int period = getAttributeInteger(node, "period", -1, Validators.GT_ZERO);
+ String unitStr = getAttributeValue(node, "unit");
+
+ if (unitStr == null) {
+ unitStr = "DAYS";
+ }
+
+ TimeUnit unit;
+
+ switch (unitStr) {
Review comment:
e.g. this switch could be simplified down to:
unit = TimeUnit.valueOf(unitStr.toUpperCase());
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632888143
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The location on your file system where the journal-retention will take place.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="period" type="xsd:integer" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ The amount of time used to keep files.
Review comment:
maybe can use xsd assert to handle more complex checks that at least size or time is set
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r633860816
##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -675,6 +675,82 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="journal-retention" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ Configuration about the journal retention configuration.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:complexType>
+ <xsd:attribute name="unit" use="required">
+ <xsd:annotation>
+ <xsd:documentation>
+ This configures the period type to use on limit. By default it is DAYS.
+ </xsd:documentation>
+ </xsd:annotation>
+ <xsd:simpleType>
+ <xsd:restriction base="xsd:string">
+ <xsd:enumeration value="DAYS"/>
+ <xsd:enumeration value="HOURS"/>
+ <xsd:enumeration value="SECONDS"/>
+ </xsd:restriction>
+ </xsd:simpleType>
+ </xsd:attribute>
+ <xsd:attribute name="directory" type="xsd:string" use="required">
Review comment:
I dont follow. Im sure if its enabled by declaring the xml were ensuring that directory is NOT optional, as such we shouldnt be defaulting the value. Just as we dont for the other four data dirs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3580: ARTEMIS-3297 Historic Journal Backup
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3580:
URL: https://github.com/apache/activemq-artemis/pull/3580#discussion_r632352294
##########
File path: artemis-server/src/test/resources/ConfigurationTest-full-config.xml
##########
@@ -406,6 +406,9 @@
<page-max-concurrent-io>17</page-max-concurrent-io>
<read-whole-page>true</read-whole-page>
<journal-directory>somedir2</journal-directory>
+ <journal-history-directory>history</journal-history-directory>
Review comment:
this config looks neat, but I think "journal-retention" is a better name, and I think in high volume scenarios second granularity will be necessary to capture use cases less than days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org