Add support for non-incremental groups push.

Add support for non-incremental groups push.

[pjo, 2015-04-24 21:59:27]

Looks close.  Tahnk you

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
File src/com/google/enterprise/adaptor/DocIdPusher.java (right):

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdPusher.java:149: boolean caseSensitive)
throws InterruptedException;
should we add a func with boolean but wiuthout error-handler?

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdPusher.java:187: */
I think this javadoc should have additional info for the new parameter?

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
File src/com/google/enterprise/adaptor/DocIdSender.java (right):

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:209: * incremental group
pushes (and batches).
Say what it does directly, without the context of 7.4 .

maybe could just @Override

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:222: */
maybe could just @Override javadoc

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:225: boolean caseSensitive,
ExceptionHandler handler, boolean incremental)
should we allow client to provide source name? i think "no".

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:290: incremental = true;
put into javadocs

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:298: final int max =
incremental ? config.getFeedMaxUrls() : defs.size();
put into javadocs

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
File src/com/google/enterprise/adaptor/GsaFeedFileSender.java (right):

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/GsaFeedFileSender.java:112: (incremental ?
"incremental" : "full"));
have you QA'ed the resulting DBs on a GSA?

[myk, 2015-04-24 23:59:06]

Addressed PJ's comments from Patch Set 1.

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
File src/com/google/enterprise/adaptor/DocIdPusher.java (right):

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdPusher.java:149: boolean caseSensitive)
throws InterruptedException;
On 2015/04/24 21:59:27, pjo wrote:
> should we add a func with boolean but wiuthout error-handler?

Done.

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdPusher.java:187: */
On 2015/04/24 21:59:27, pjo wrote:
> I think this javadoc should have additional info for the new parameter?

Done.

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
File src/com/google/enterprise/adaptor/DocIdSender.java (right):

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:209: * incremental group
pushes (and batches).
On 2015/04/24 21:59:27, pjo wrote:
> Say what it does directly, without the context of 7.4 .
> 
> maybe could just @Override

Removed the javadocs (to inherit from interface).

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:222: */
On 2015/04/24 21:59:27, pjo wrote:
> maybe could just @Override javadoc

here, too

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:225: boolean caseSensitive,
ExceptionHandler handler, boolean incremental)
On 2015/04/24 21:59:27, pjo wrote:
> should we allow client to provide source name? i think "no".

It's the feed name.  We don't need to provide another name for the admin to
(mis?)-configure.

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:290: incremental = true;
On 2015/04/24 21:59:27, pjo wrote:
> put into javadocs

Done.

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/DocIdSender.java:298: final int max =
incremental ? config.getFeedMaxUrls() : defs.size();
On 2015/04/24 21:59:27, pjo wrote:
> put into javadocs

Done.

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
File src/com/google/enterprise/adaptor/GsaFeedFileSender.java (right):

https://codereview.appspot.com/222480044/diff/1/src/com/google/enterprise/ada...
src/com/google/enterprise/adaptor/GsaFeedFileSender.java:112: (incremental ?
"incremental" : "full"));
On 2015/04/24 21:59:27, pjo wrote:
> have you QA'ed the resulting DBs on a GSA?

Not yet.  Was planning on doing so with AD.

[pjo, 2015-04-25 18:55:29]

LGTM.  Thank you

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
File src/com/google/enterprise/adaptor/DocIdPusher.java (right):

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdPusher.java:146: */
change javadoc to say that push is incremental and refer to new decl

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdPusher.java:166: */
change javadoc to say that push is incremental and refer to new decl

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdPusher.java:221: * the groups specified
in <code>defs</code>.
copy param javadoc from above decl.

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
File src/com/google/enterprise/adaptor/DocIdSender.java (right):

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdSender.java:294: // all groups from the
given data source), we must do it in a single "batch"
give the reason: because otherwise a failure in later batch would mean
incomplete state without ability to backout. so one batch in order to make sure
all or nothing.

[ondrejnovak, 2015-04-27 06:22:27]

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
File src/com/google/enterprise/adaptor/DocIdSender.java (right):

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdSender.java:295: final int max =
incremental ? config.getFeedMaxUrls() : defs.size();
Building the group XML is a very memory hungry operation, can you log here

1. that we are doing non-incremental group push
2. the value of defs.size()

, so that it's among the last log lines before we run out of memory

[myk, 2015-04-27 22:09:22]

Addressed comments from PJ and from Ondra.  (Ondra, please take a look at the
new log message.)

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
File src/com/google/enterprise/adaptor/DocIdPusher.java (right):

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdPusher.java:146: */
On 2015/04/25 18:55:28, pjo wrote:
> change javadoc to say that push is incremental and refer to new decl

Done.

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdPusher.java:166: */
On 2015/04/25 18:55:28, pjo wrote:
> change javadoc to say that push is incremental and refer to new decl

Done.

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdPusher.java:221: * the groups specified
in <code>defs</code>.
On 2015/04/25 18:55:28, pjo wrote:
> copy param javadoc from above decl.

Done.

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
File src/com/google/enterprise/adaptor/DocIdSender.java (right):

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdSender.java:294: // all groups from the
given data source), we must do it in a single "batch"
On 2015/04/25 18:55:29, pjo wrote:
> give the reason: because otherwise a failure in later batch would mean
> incomplete state without ability to backout. so one batch in order to make
sure
> all or nothing.

Done.

https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
src/com/google/enterprise/adaptor/DocIdSender.java:295: final int max =
incremental ? config.getFeedMaxUrls() : defs.size();
On 2015/04/27 06:22:27, ondrejnovak wrote:
> Building the group XML is a very memory hungry operation, can you log here
> 
> 1. that we are doing non-incremental group push
> 2. the value of defs.size()
> 
> , so that it's among the last log lines before we run out of memory

Done.  I was considering adding instructions on what-to-do-upon-an-OOM here, but
both John Felton and PJ felt this should not occur every time we try this
operation.  Please let me know if you are OK with the log message as is. 
(Originally, I was going to put this message (about needing lots of memory) in
the AD Adaptor, but it's also possible for the SharePoint Adaptor to run out of
memory here, so this is the better place for it.

[myk, 2015-04-28 16:29:48]

Just a ping -- I'm hoping to submit this today so that I can start on the
AD side of this.
            -- Marc

On Mon, Apr 27, 2015 at 3:09 PM, <myk@google.com> wrote:

> Addressed comments from PJ and from Ondra.  (Ondra, please take a look
> at the new log message.)
>
>
>
>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
> File src/com/google/enterprise/adaptor/DocIdPusher.java (right):
>
>
>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
> src/com/google/enterprise/adaptor/DocIdPusher.java:146: */
> On 2015/04/25 18:55:28, pjo wrote:
>
>> change javadoc to say that push is incremental and refer to new decl
>>
>
> Done.
>
>
>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
> src/com/google/enterprise/adaptor/DocIdPusher.java:166: */
> On 2015/04/25 18:55:28, pjo wrote:
>
>> change javadoc to say that push is incremental and refer to new decl
>>
>
> Done.
>
>
>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
> src/com/google/enterprise/adaptor/DocIdPusher.java:221: * the groups
> specified in <code>defs</code>.
> On 2015/04/25 18:55:28, pjo wrote:
>
>> copy param javadoc from above decl.
>>
>
> Done.
>
>
>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
> File src/com/google/enterprise/adaptor/DocIdSender.java (right):
>
>
>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
> src/com/google/enterprise/adaptor/DocIdSender.java:294: // all groups
> from the given data source), we must do it in a single "batch"
> On 2015/04/25 18:55:29, pjo wrote:
>
>> give the reason: because otherwise a failure in later batch would mean
>> incomplete state without ability to backout. so one batch in order to
>>
> make sure
>
>> all or nothing.
>>
>
> Done.
>
>
>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
> src/com/google/enterprise/adaptor/DocIdSender.java:295: final int max =
> incremental ? config.getFeedMaxUrls() : defs.size();
> On 2015/04/27 06:22:27, ondrejnovak wrote:
>
>> Building the group XML is a very memory hungry operation, can you log
>>
> here
>
>  1. that we are doing non-incremental group push
>> 2. the value of defs.size()
>>
>
>  , so that it's among the last log lines before we run out of memory
>>
>
> Done.  I was considering adding instructions on what-to-do-upon-an-OOM
> here, but both John Felton and PJ felt this should not occur every time
> we try this operation.  Please let me know if you are OK with the log
> message as is.  (Originally, I was going to put this message (about
> needing lots of memory) in the AD Adaptor, but it's also possible for
> the SharePoint Adaptor to run out of memory here, so this is the better
> place for it.
>
> https://codereview.appspot.com/222480044/
>

[ondrejnovak, 2015-04-28 21:38:41]

Could you add the data source name in the message. Otherwise it looks good
Dne 28. 4. 2015 5:29 PM napsal uživatel "Marc Kriguer" <myk@google.com>:

> Just a ping -- I'm hoping to submit this today so that I can start on the
> AD side of this.
>             -- Marc
>
> On Mon, Apr 27, 2015 at 3:09 PM, <myk@google.com> wrote:
>
>> Addressed comments from PJ and from Ondra.  (Ondra, please take a look
>> at the new log message.)
>>
>>
>>
>>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
>> File src/com/google/enterprise/adaptor/DocIdPusher.java (right):
>>
>>
>>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
>> src/com/google/enterprise/adaptor/DocIdPusher.java:146: */
>> On 2015/04/25 18:55:28, pjo wrote:
>>
>>> change javadoc to say that push is incremental and refer to new decl
>>>
>>
>> Done.
>>
>>
>>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
>> src/com/google/enterprise/adaptor/DocIdPusher.java:166: */
>> On 2015/04/25 18:55:28, pjo wrote:
>>
>>> change javadoc to say that push is incremental and refer to new decl
>>>
>>
>> Done.
>>
>>
>>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
>> src/com/google/enterprise/adaptor/DocIdPusher.java:221: * the groups
>> specified in <code>defs</code>.
>> On 2015/04/25 18:55:28, pjo wrote:
>>
>>> copy param javadoc from above decl.
>>>
>>
>> Done.
>>
>>
>>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
>> File src/com/google/enterprise/adaptor/DocIdSender.java (right):
>>
>>
>>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
>> src/com/google/enterprise/adaptor/DocIdSender.java:294: // all groups
>> from the given data source), we must do it in a single "batch"
>> On 2015/04/25 18:55:29, pjo wrote:
>>
>>> give the reason: because otherwise a failure in later batch would mean
>>> incomplete state without ability to backout. so one batch in order to
>>>
>> make sure
>>
>>> all or nothing.
>>>
>>
>> Done.
>>
>>
>>
https://codereview.appspot.com/222480044/diff/20001/src/com/google/enterprise...
>> src/com/google/enterprise/adaptor/DocIdSender.java:295: final int max =
>> incremental ? config.getFeedMaxUrls() : defs.size();
>> On 2015/04/27 06:22:27, ondrejnovak wrote:
>>
>>> Building the group XML is a very memory hungry operation, can you log
>>>
>> here
>>
>>  1. that we are doing non-incremental group push
>>> 2. the value of defs.size()
>>>
>>
>>  , so that it's among the last log lines before we run out of memory
>>>
>>
>> Done.  I was considering adding instructions on what-to-do-upon-an-OOM
>> here, but both John Felton and PJ felt this should not occur every time
>> we try this operation.  Please let me know if you are OK with the log
>> message as is.  (Originally, I was going to put this message (about
>> needing lots of memory) in the AD Adaptor, but it's also possible for
>> the SharePoint Adaptor to run out of memory here, so this is the better
>> place for it.
>>
>> https://codereview.appspot.com/222480044/
>>
>
>