Making the content syncs more resilient by mzgubin · Pull Request #214 · TWCable/grabbit

mzgubin · 2018-08-29T15:18:37Z

Changes that improve resiliency like catching some of the errors encountered to keep the jobs running. It would be great if that was perhaps put into a configuration whether to fail-fast or just log those errors as warnings and go on. I don't know enough about the best approach for something like that would be.
Converted the servlet on the source being pulled from to handle post requests instead of get. That way the requests won't be limited by the GET URL length. This can be useful if there are a ton of exclusions as we've encountered.
Also I made a change to how the exclusion filters are matching the paths. Currently the paths will be searched to find the exclusion string in any part of the path, instead I've made it so that the string is matched against just the startsWith instead. Not sure if that's how it's intended to be used, but perhaps that can also be configured somehow in the exclusion filter section itself.
Added more debug logging

* Continuing on error when writing nodes on the client (catching and logging the ConstraintViolationException and the InvalidItemStateException)

…Logging the errors. Going to see if this helps with giving more information when something goes wrong on the server side like reading too large of a file.

…s in org.apache.jackrabbit.oak.plugins.blob.datastore.DataStoreBlobStore on line 676 of oak-blob-plugins-1.8.2.jar

… post method so that we are not limited by URL length in a GET call. This way we can pass a large exclusion list.

…re-logging Fixes/continue on error and more logging

…rt of the path to see what it starts with. We see if it matches or if it starts with that path (plus a forward slash to denote that it's a parent path)

Fixing the ExclusionPathNodeIterator to not just look at the first pa…

# Conflicts: # gradle.properties # src/main/groovy/com/twcable/grabbit/server/batch/steps/jcrnodes/JcrNodesWriter.groovy # src/main/groovy/com/twcable/grabbit/server/services/ExcludePathNodeIterator.groovy

sagarsane · 2018-09-10T02:38:15Z

gradle.properties


 # Please keep alphabetical
+apache_httpclient_version = 4.5.4
+apache_httpcore_version = 4.4.8


Since we are using the OkHttpClient already, it would be better to continue to use that than having another way to create connections using the Apache HttpClient.

Ok will switch to that one

sagarsane · 2018-09-10T02:38:20Z

gradle.properties

 slf4j_version = 1.7.6
 sling_api_version = 2.9.0
 sling_base_version = 2.2.2
+sling_commons_json_version = 2.0.6


It would be better to continue using Groovy's built-in JSONBuilder for consistency as used elsewhere in the codebase rather than adding this new dependency!

Sure, I'll switch it

sagarsane · 2018-09-10T02:38:38Z

src/main/groovy/com/twcable/grabbit/server/GrabbitContentPullServlet.groovy

+                    "path={}\nafter={}\nexcludePathList={}\n\n\n", path,
+                    afterDateString, StringUtils.join(excludePathsList, ",")
+
+            serverService.getContentForRootPath(serverUsername, path, excludePathsList ?: null,


I agree the need to have a POST here instead of GET due to GET's limitations. However, since this is an internal call (from Grabbit client to Grabbit server), I don't think we need to keep both GET and POST.

I would thus suggest removing doGet() and also updating the GrabbitContentPullServletSpec accordingly.

sagarsane · 2018-09-10T02:41:43Z

src/main/groovy/com/twcable/grabbit/server/services/impl/TreeTraverser.java

@@ -0,0 +1,344 @@
+package com.twcable.grabbit.server.services.impl;


Hi @mzgubin I did not see info about this in your PR description. Could you please let us know the need to re-implement this instead of using the TreeTraverser from Jackrabbit commons as it was before?

Well the reason was that it allowed me to log what the error was when it was traversing. By default it was set to ErrorHandler.IGNORE so the errors were just being swallowed up.

sagarsane · 2018-09-10T02:42:13Z

src/main/groovy/com/twcable/grabbit/server/batch/steps/jcrnodes/JcrNodesWriter.groovy

-            log.debug "Sending NodeProto : ${node.name}"
-            node.writeDelimitedTo(servletOutputStream)
+        try {
+            //log.info "\n\n### NodeProtos ###\n\n${StringUtils.join(nodeProtos.toString(), "\n")}"


We can get rid of all the commented out code (here and elsewhere in the PR). Thanks!

No problem will do

mzgubin · 2018-10-04T22:18:07Z

Sorry have been bogged down but will definitely get to this in the next week

- Removing the commons sling json dependencies and using the the google groovy builder for json - Removing commented out code - Removing the GET method from the GrabbitContentPullServlet

Maxim Gubin and others added 18 commits June 6, 2018 11:38

* Added more logging

1807ca6

* Continuing on error when writing nodes on the client (catching and logging the ConstraintViolationException and the InvalidItemStateException)

Catching more exceptions when writing on the client

a7f1e02

Catching more exceptions when saving properties on a node on the client

9147c5e

Catching errors for when reading and writing of nodes on the server. …

f594a80

…Logging the errors. Going to see if this helps with giving more information when something goes wrong on the server side like reading too large of a file.

Fixing the large file transfer issue

dfa0345

Catching more errors including NullPointer that we've seen that occur…

9a1b08c

…s in org.apache.jackrabbit.oak.plugins.blob.datastore.DataStoreBlobStore on line 676 of oak-blob-plugins-1.8.2.jar

Adding exception stacktraces

cf3f36b

Making the call to the server via the GrabbitContentPullServlet use a…

3b3ae11

… post method so that we are not limited by URL length in a GET call. This way we can pass a large exclusion list.

Merge pull request #1 from HBOCodeLabs/fixes/continue-on-error-and-mo…

f282a21

…re-logging Fixes/continue on error and more logging

Fixing the ExclusionPathNodeIterator to not just look at the first pa…

503c93f

…rt of the path to see what it starts with. We see if it matches or if it starts with that path (plus a forward slash to denote that it's a parent path)

Merge pull request #2 from HBOCodeLabs/fixes/fix-exclusion-filter

e749372

Fixing the ExclusionPathNodeIterator to not just look at the first pa…

Merge branch 'master' of https://github.com/TWCable/grabbit

509fab0

# Conflicts: # gradle.properties # src/main/groovy/com/twcable/grabbit/server/batch/steps/jcrnodes/JcrNodesWriter.groovy # src/main/groovy/com/twcable/grabbit/server/services/ExcludePathNodeIterator.groovy

Updating the version to 7.1.6

5e49b0f

Fixing a merge issue

ae61e27

Fixing failing unit tests

b85d2b6

Cleaning up some imports

5b2aeee

Keeping alphabetical

0e92bc8

Adding the required imports back

01e1623

mzgubin mentioned this pull request Aug 30, 2018

Be able to specify "continue on error" #69

Open

sagarsane requested a review from jbornemann September 10, 2018 02:03

sagarsane reviewed Sep 10, 2018

View reviewed changes

Maxim Gubin added 2 commits January 10, 2019 13:07

- Removing the apache httpclient dependencies

aefb204

- Removing the commons sling json dependencies and using the the google groovy builder for json - Removing commented out code - Removing the GET method from the GrabbitContentPullServlet

Fixing up some errors

bd8e0ff

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making the content syncs more resilient#214

Making the content syncs more resilient#214
mzgubin wants to merge 20 commits intoTWCable:masterfrom
HBOCodeLabs:master

mzgubin commented Aug 29, 2018 •

edited

Loading

Uh oh!

sagarsane Sep 10, 2018

Uh oh!

mzgubin Sep 10, 2018

Uh oh!

sagarsane Sep 10, 2018

Uh oh!

mzgubin Sep 10, 2018

Uh oh!

sagarsane Sep 10, 2018

Uh oh!

mzgubin Sep 10, 2018

Uh oh!

sagarsane Sep 10, 2018

Uh oh!

mzgubin Jan 10, 2019

Uh oh!

sagarsane Sep 10, 2018

Uh oh!

mzgubin Sep 10, 2018

Uh oh!

mzgubin commented Oct 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,344 @@
		package com.twcable.grabbit.server.services.impl;

Conversation

mzgubin commented Aug 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzgubin commented Oct 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mzgubin commented Aug 29, 2018 •

edited

Loading