gsmiller commented on code in PR #974:
URL: https://github.com/apache/lucene/pull/974#discussion_r914083105
##########
lucene/demo/src/java/org/apache/lucene/demo/facet/RangeFacetsExample.java:
##########
@@ -73,6 +76,30 @@ public void index() throws IOException {
indexWriter.addDocument(doc);
}
+ // Add documents with a fake timestamp, 3600 sec (1 hour) before "now",
7200 sec before (2
+ // hours) before "now", ...:
+ long currentTime = System.currentTimeMillis() / 1000L;
+ // Index error messages for the past week (24 * 7 = 168 hours)
+ for (int i = 0; i < 168; i++) {
+ long then = currentTime - (i + 1) * 3600;
+
+ // conditionally add different number of error messages to the past hour
slot
+ for (int j = 0; j < i % 35; j++) {
Review Comment:
Why mod with a value of `35`? I'm not getting it. Is there a specific reason
you're using the value `35`? If so, could you add a comment?
##########
lucene/demo/src/java/org/apache/lucene/demo/facet/RangeFacetsExample.java:
##########
@@ -73,6 +76,30 @@ public void index() throws IOException {
indexWriter.addDocument(doc);
}
+ // Add documents with a fake timestamp, 3600 sec (1 hour) before "now",
7200 sec before (2
+ // hours) before "now", ...:
+ long currentTime = System.currentTimeMillis() / 1000L;
+ // Index error messages for the past week (24 * 7 = 168 hours)
+ for (int i = 0; i < 168; i++) {
Review Comment:
minor: I find it a bit confusing that you index the data and create ranges
in "reverse" starting from "now." Would it be easier for others to understand
if you started "one week ago" and looped "forwards"? Since the demo code is
here to help people understand the functionality, I want to make sure we don't
create unnecessary confusion.
##########
lucene/demo/src/java/org/apache/lucene/demo/facet/RangeFacetsExample.java:
##########
@@ -73,6 +76,30 @@ public void index() throws IOException {
indexWriter.addDocument(doc);
}
+ // Add documents with a fake timestamp, 3600 sec (1 hour) before "now",
7200 sec before (2
+ // hours) before "now", ...:
+ long currentTime = System.currentTimeMillis() / 1000L;
+ // Index error messages for the past week (24 * 7 = 168 hours)
+ for (int i = 0; i < 168; i++) {
+ long then = currentTime - (i + 1) * 3600;
+
+ // conditionally add different number of error messages to the past hour
slot
+ for (int j = 0; j < i % 35; j++) {
+ Document doc = new Document();
+ doc.add(new NumericDocValuesField("error log", then));
Review Comment:
Could we add some "jitter" to what gets indexed so all the timestamps of the
"fake error logs" don't fall right on hour boundaries? That would be a bit of a
more realistic example.
##########
lucene/demo/src/java/org/apache/lucene/demo/facet/RangeFacetsExample.java:
##########
@@ -73,6 +76,30 @@ public void index() throws IOException {
indexWriter.addDocument(doc);
}
+ // Add documents with a fake timestamp, 3600 sec (1 hour) before "now",
7200 sec before (2
+ // hours) before "now", ...:
+ long currentTime = System.currentTimeMillis() / 1000L;
+ // Index error messages for the past week (24 * 7 = 168 hours)
+ for (int i = 0; i < 168; i++) {
+ long then = currentTime - (i + 1) * 3600;
+
+ // conditionally add different number of error messages to the past hour
slot
+ for (int j = 0; j < i % 35; j++) {
+ Document doc = new Document();
+ doc.add(new NumericDocValuesField("error log", then));
+ doc.add(
+ new StringField(
+ "Error msg", "[Error] Server encountered error at " +
currentTime, Field.Store.NO));
Review Comment:
Shouldn't the "logged" timestamp here be the same as the one indexed in the
previous line (i.e., `then` instead of `currentTime`)? I realize that doesn't
impact the functionality of the example, but just trying to avoid confusion.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
