stefan-egli commented on code in PR #1422:
URL: https://github.com/apache/jackrabbit-oak/pull/1422#discussion_r1567100681
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java:
##########
@@ -226,6 +236,46 @@ public Optional<NodeDocument> getOldestModifiedDoc(final
Clock clock) {
return empty();
}
+ /**
+ * Retrieve the document with given id with only required fields.
+ *
+ * @param id the document id
+ * @param fields {@link List} of required fields, keep empty to fetch all
+ *
+ * @return the document with given id or empty if not found
+ */
+ public Optional<NodeDocument> getDocument(final String id, final
List<String> fields) {
+
+ Iterable<NodeDocument> docs = null;
+ try {
+ docs = stream(getSelectedDocuments(store, null, 0,
MIN_ID_VALUE).spliterator(), false)
+ .filter(input -> idEquals(input,
id)).limit(1).collect(toList());
+ if (docs.iterator().hasNext()) {
+ final NodeDocument doc = docs.iterator().next();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Found Document with id {}", id);
+ }
+ if (fields == null || fields.isEmpty()) {
+ return ofNullable(doc);
+ }
+
+ final Set<String> projectedSet = newHashSet(fields);
+ projectedSet.add(ID);
+
+ final NodeDocument newDoc =
Collection.NODES.newDocument(store);
+ doc.deepCopy(newDoc);
+ newDoc.keySet().retainAll(projectedSet);
+ return of(newDoc);
+ }
+
+ } finally {
+ Utils.closeIfCloseable(docs);
+ }
+
+ LOG.info("No Doc has been found with id [{}]", id);
Review Comment:
Shouldn't this be eg `debug`?
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##########
@@ -1216,34 +1220,27 @@ private boolean isDeletedOrOrphanedNode(final NodeState
traversedState,
phases.stop(GCPhase.DETAILED_GC_COLLECT_ORPHAN_NODES);
return false;
}
- if (detailedGcMode == DetailedGCMode.GAP_ORPHANS
- || detailedGcMode ==
DetailedGCMode.GAP_ORPHANS_EMPTYPROPS) {
+ if (detailedGcMode == DetailedGCMode.GAP_ORPHANS || detailedGcMode
== DetailedGCMode.GAP_ORPHANS_EMPTYPROPS) {
// check the ancestor docs for gaps
final Path docPath = doc.getPath();
- final Path geaPath = greatestExistingAncestorOrSelf;
- final Path geaChildPath = docPath
- .getAncestor(docPath.getDepth() - geaPath.getDepth() -
1);
+ final Path geaChildPath =
docPath.getAncestor(docPath.getDepth() -
greatestExistingAncestorOrSelf.getDepth() - 1);
Review Comment:
no strong opinion, but why was this changed?
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##########
@@ -1216,34 +1220,27 @@ private boolean isDeletedOrOrphanedNode(final NodeState
traversedState,
phases.stop(GCPhase.DETAILED_GC_COLLECT_ORPHAN_NODES);
return false;
}
- if (detailedGcMode == DetailedGCMode.GAP_ORPHANS
- || detailedGcMode ==
DetailedGCMode.GAP_ORPHANS_EMPTYPROPS) {
+ if (detailedGcMode == DetailedGCMode.GAP_ORPHANS || detailedGcMode
== DetailedGCMode.GAP_ORPHANS_EMPTYPROPS) {
// check the ancestor docs for gaps
final Path docPath = doc.getPath();
- final Path geaPath = greatestExistingAncestorOrSelf;
- final Path geaChildPath = docPath
- .getAncestor(docPath.getDepth() - geaPath.getDepth() -
1);
+ final Path geaChildPath =
docPath.getAncestor(docPath.getDepth() -
greatestExistingAncestorOrSelf.getDepth() - 1);
Boolean missingType = missingDocsTypes.get(geaChildPath);
if (missingType == null) {
// we don't have it cached yet - so do the potentially
expensive find
- final NodeDocument d =
nodeStore.getDocumentStore().find(NODES,
- Utils.getIdFromPath(geaChildPath));
- final boolean parentDocExists = d != null;
- missingType = !parentDocExists;
+ missingType =
versionStore.getDocument(getIdFromPath(geaChildPath), of(ID)).isEmpty();
if (missingDocsTypes.size() >
DETAILED_GC_MISSING_DOCS_TYPE_CACHE_SIZE) {
final Iterator<Path> it =
missingDocsTypes.keySet().iterator();
it.next();
it.remove();
if (missingDocsTypes.size() >
DETAILED_GC_MISSING_DOCS_TYPE_CACHE_SIZE) {
// should never really happen, if it does: clear
all, break out
- log.error("isDeletedOrOrphanedNode : knownNullDocs
removal failed, size was {}",
- missingDocsTypes.size());
+ log.error("isDeletedOrOrphanedNode : knownNullDocs
removal failed, size was {}", missingDocsTypes.size());
missingDocsTypes.clear();
}
}
missingDocsTypes.put(geaChildPath, missingType);
}
- if (missingType == false) {
+ if (!missingType) {
Review Comment:
reason for the non typical notation was that `missingType` had 3 states:
null/true/false, and it being false was one of the types.
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##########
@@ -1216,34 +1220,27 @@ private boolean isDeletedOrOrphanedNode(final NodeState
traversedState,
phases.stop(GCPhase.DETAILED_GC_COLLECT_ORPHAN_NODES);
return false;
}
- if (detailedGcMode == DetailedGCMode.GAP_ORPHANS
- || detailedGcMode ==
DetailedGCMode.GAP_ORPHANS_EMPTYPROPS) {
+ if (detailedGcMode == DetailedGCMode.GAP_ORPHANS || detailedGcMode
== DetailedGCMode.GAP_ORPHANS_EMPTYPROPS) {
// check the ancestor docs for gaps
final Path docPath = doc.getPath();
- final Path geaPath = greatestExistingAncestorOrSelf;
- final Path geaChildPath = docPath
- .getAncestor(docPath.getDepth() - geaPath.getDepth() -
1);
+ final Path geaChildPath =
docPath.getAncestor(docPath.getDepth() -
greatestExistingAncestorOrSelf.getDepth() - 1);
Boolean missingType = missingDocsTypes.get(geaChildPath);
if (missingType == null) {
// we don't have it cached yet - so do the potentially
expensive find
- final NodeDocument d =
nodeStore.getDocumentStore().find(NODES,
- Utils.getIdFromPath(geaChildPath));
- final boolean parentDocExists = d != null;
- missingType = !parentDocExists;
Review Comment:
no strong opinion, but why was this changed?
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java:
##########
@@ -165,6 +166,27 @@ public Iterable<NodeDocument> getModifiedDocs(final long
fromModified, final lon
return wrap(transform(cursor, input ->
store.convertFromDBObject(NODES, input)));
}
+ @Override
+ public Optional<NodeDocument> getDocument(final String id, final
List<String> fields) {
+
+ final Bson query = eq(ID, id);
+
+ final FindIterable<BasicDBObject> result =
getNodeCollection().find(query);
+
+ if (fields != null && !fields.isEmpty()) {
+ result.projection(include(fields));
+ }
+
+ try(MongoCursor<BasicDBObject> cur = result.iterator()) {
+ return cur.hasNext() ? ofNullable(store.convertFromDBObject(NODES,
cur.next())) : empty();
+ } catch (Exception ex) {
+ LOG.error("getDocument() <- error while fetching data from Mongo",
ex);
+ }
+ LOG.info("No Doc has been found with id [{}], retuning empty", id);
Review Comment:
same as the other, shouldn't this be eg `debug`?
--
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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
