TDL/7493 Make BagGenerator threads configurable #8606

qqmyers · 2022-04-12T23:24:20Z

What this PR does / why we need it: Improves scaling of Archiving and allows it to use more or fewer threads depending on machine size

Which issue(s) this PR closes:

Closes #8602

Special notes for your reviewer:

Suggestions on how to test this: Configure archiving (e.g. with LocalSubmitToArchiveCommand), change the :BagGeneratorThreads setting and compare the load and/or watch the log to see how many files are being zipped in parallel.

Does this PR introduce a user interface change? If mockups are available, please link/include them here: No

Is there a release notes update needed for this change?: multiple Bag/Archiving changes coming - perhaps one overall release note?

Additional documentation: guides updated

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

…om/TexasDigitalLibrary/dataverse.git into IQSS/7493-batch_archiving_API_call

…iving_API_call

…ving_API_call

checks to see if space exists

…ving_API_call

IQSS/8422-default_to_allow_dataset_level_choice_of_metadatalanguage

TDL/7493-batch_archiving_API_call Conflicts: modules/dataverse-parent/pom.xml

…erate_threads_configurable_only

coveralls · 2022-04-12T23:27:09Z

Coverage increased (+0.002%) to 18.999% when pulling 32f3a58 on TexasDigitalLibrary:TDL/7493-make_BagGenerate_threads_configurable_only into a5f703e on IQSS:develop.

pdurbin

Overall this looks good. It seems like all the ParallelScatterZipCreator code is already in place and what this pull request is doing is reducing the default number of threads from 8 to 2 and making it configurable.

I didn't actually run the code but I navigated around it a bit.

I found a few small things we should probably fix up so I'm hitting "request changes" for now. Details are in the review.

doc/sphinx-guides/source/developers/workflows.rst

pdurbin · 2022-04-13T19:55:46Z

doc/sphinx-guides/source/developers/workflows.rst

@@ -180,7 +180,7 @@ archiver

 A step that sends an archival copy of a Dataset Version to a configured archiver, e.g. the DuraCloud interface of Chronopolis. See the `DuraCloud/Chronopolis Integration documentation <http://guides.dataverse.org/en/latest/admin/integrations.html#id15>`_ for further detail.


It might be nice to fix this "#id15" link while were in here. (It links to the table of contents rather than the actual content below the toc.) We could add a custom anchor and use :ref: to link to it.

doc/sphinx-guides/source/installation/config.rst

pdurbin · 2022-04-13T20:17:27Z

src/main/java/edu/harvard/iq/dataverse/util/bagit/BagGenerator.java

@@ -115,6 +115,7 @@ public class BagGenerator {
    private boolean usetemp = false;

    private int numConnections = 8;
+    public static final String BAG_GENERATOR_THREADS = ":BagGeneratorThreads";


Instead of having a String here, can we keep all the database settings centralized in the Key enum at SettingsServiceBean?

I tried this locally and the following seems to work:

public static final String BAG_GENERATOR_THREADS = SettingsServiceBean.Key.BagGeneratorThreads.toString();

(I had to import SettingsServiceBean.)

I see that DuraCloudSubmitToArchiveCommand also has strings like :DuraCloudPort, :DuraCloudHost, and :DuraCloudContext hard-coded, but we could centralize those later.

I've purposely avoided that so far with the hope that at some point these archivers will be packaged separately. Right now they are loaded via reflection (you specify the classname in a property) and there should be no references to the specific Archiver classes or their properties in the main codebase. (As you note though we do have their properties in the master list in the guides so far.)
( I hope it isn't too much work for these classes and their dependencies to be pulled out into separate jars but it isn't something I know how to do off-hand - perhaps @poikilotherm?)

Interesting. I guess I was under the impression that since these archiver classes/commands are in the main code base that they have to be.

It would be nice to figure out how to extract them to a different git repo. It would be a great success story of modularity in Dataverse.

Once they are truly extracted from the main code base, I suppose their documentation should be extracted too, just like we do with external tools.

It sounds like we're not there yet. That's fine. Thanks for the cleanup you did.

pdurbin · 2022-04-14T13:56:59Z

doc/sphinx-guides/source/admin/integrations.rst


-A Dataverse installation can be configured to submit a copy of published Datasets, packaged as `Research Data Alliance conformant <https://www.rd-alliance.org/system/files/Research%20Data%20Repository%20Interoperability%20WG%20-%20Final%20Recommendations_reviewed_0.pdf>`_ zipped `BagIt <https://tools.ietf.org/html/draft-kunze-bagit-17>`_ bags to the `Chronopolis <https://libraries.ucsd.edu/chronopolis/>`_ via `DuraCloud <https://duraspace.org/duracloud/>`_
+A Dataverse installation can be configured to submit a copy of published Datasets, packaged as `Research Data Alliance conformant <https://www.rd-alliance.org/system/files/Research%20Data%20Repository%20Interoperability%20WG%20-%20Final%20Recommendations_reviewed_0.pdf>`_ zipped `BagIt <https://tools.ietf.org/html/draft-kunze-bagit-17>`_ bags to the `Chronopolis <https://libraries.ucsd.edu/chronopolis/>`_ via `DuraCloud <https://duraspace.org/duracloud/>`_, to a local file system, or to `Google Cloud Storage<https://cloud.google.com/storage>`_.


This is failing with:

/github/workspace/doc/sphinx-guides/source/admin/integrations.rst:165:Unknown target name: "google cloud storagehttps://cloud.google.com/storage".

pdurbin · 2022-04-14T14:05:02Z

src/main/java/edu/harvard/iq/dataverse/util/bagit/BagGenerator.java

@@ -115,6 +115,7 @@ public class BagGenerator {
    private boolean usetemp = false;

    private int numConnections = 8;
+    public static final String BAG_GENERATOR_THREADS = ":BagGeneratorThreads";


Interesting. I guess I was under the impression that since these archiver classes/commands are in the main code base that they have to be.

It would be nice to figure out how to extract them to a different git repo. It would be a great success story of modularity in Dataverse.

Once they are truly extracted from the main code base, I suppose their documentation should be extracted too, just like we do with external tools.

It sounds like we're not there yet. That's fine. Thanks for the cleanup you did.

doc/sphinx-guides/source/developers/workflows.rst

pdurbin

Once we fix make html ( TexasDigitalLibrary#2 ) I'm ready to move this to QA. (I did also throw a wording suggestion in this review.)

I haven't tested this at all but the changes make sense and are pretty minor. Again, we're reducing the default number of threads from 8 to 2 and making it configurable.

doc/sphinx-guides/source/admin/integrations.rst

doc/sphinx-guides/source/developers/workflows.rst

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

…ttps://github.com/TexasDigitalLibrary/dataverse.git into TDL/7493-make_BagGenerate_threads_configurable_only

pdurbin

Looks good! Again, I haven't tested it but the changes make sense.

qqmyers and others added 30 commits December 23, 2020 11:38

implement batch processing of new versions to archive

6eae5e4

add listonly and limit options, count commandEx as failure

8313404

send list in response for listonly

70d923a

fix query

96d3723

case sensitive in query

cb9f374

param to only archive latest version

76e2396

off by one in limit

2e8d990

documentation

b796833

Update doc/sphinx-guides/source/installation/config.rst

006a4ba

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

Update doc/sphinx-guides/source/installation/config.rst

bba8ba0

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

Update doc/sphinx-guides/source/installation/config.rst

011c97a

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

updates per review

1a1c28c

Merge branch 'IQSS/7493-batch_archiving_API_call' of https://github.c…

8a0ad71

…om/TexasDigitalLibrary/dataverse.git into IQSS/7493-batch_archiving_API_call

Merge remote-tracking branch 'IQSS/develop' into IQSS/7493-batch_arch…

7b5aead

…iving_API_call

Merge remote-tracking branch 'IQSS/develop' into IQSS/7493-batch_arch…

fd32dfd

…iving_API_call

Merge remote-tracking branch 'IQSS/develop' into IQSS/7493-batch_arch…

805ff95

…iving_API_call

Merge remote-tracking branch 'IQSS/develop' into IQSS/7493-batch_arch…

e1415f9

…iving_API_call

Merge remote-tracking branch 'IQSS/develop' into IQSS/7493-batch_arch…

ef9a0b9

…iving_API_call

Merge remote-tracking branch 'IQSS/develop' into IQSS/7493-batch_arch…

9443e04

…iving_API_call

Merge remote-tracking branch 'IQSS/develop' into IQSS/7493-batch_arch…

242befa

…iving_API_call

Merge remote-tracking branch 'IQSS/develop' into TDL/7493-batch_archi…

7047d00

…ving_API_call

TDL Bundle text

4549f0c

fix thread use of requestscoped service

56ff7bc

update doc to match api call name

46f8554

adjust to use a space per dataverse (alias)

33b85f4

checks to see if space exists

Merge remote-tracking branch 'IQSS/develop' into TDL/7493-batch_archi…

a5edb8c

…ving_API_call

custom version

e205f4b

Merge remote-tracking branch 'IQSS/develop' into

5e625a5

IQSS/8422-default_to_allow_dataset_level_choice_of_metadatalanguage

Merge remote-tracking branch 'IQSS/develop' into

53b9803

TDL/7493-batch_archiving_API_call Conflicts: modules/dataverse-parent/pom.xml

Use try with resources to close connections for non-200 status

30877d7

qqmyers added 3 commits April 9, 2022 18:24

remove all but thread code, add Google/Local archivers

48dd0e9

Merge remote-tracking branch 'IQSS/develop' into TDL/7493-make_BagGen…

4a75c93

…erate_threads_configurable_only

document setting/function

3dfb0c3

qqmyers added the GDCC: TDL supported by Texas Digital Library label Apr 12, 2022

qqmyers changed the title ~~Tdl/7493 make bag generate threads configurable only~~ TDL/7493 Make BagGenerator threads configurable Apr 12, 2022

scolapasta assigned pdurbin Apr 13, 2022

pdurbin requested changes Apr 13, 2022

View reviewed changes

pdurbin assigned qqmyers Apr 13, 2022

documentation update per review

cd7602b

qqmyers removed their assignment Apr 13, 2022

qqmyers mentioned this pull request Apr 13, 2022

Archiving/Bag/OAI_ORE/DataCommons Release notes #8611

Closed

pdurbin reviewed Apr 14, 2022

View reviewed changes

add required space

36aa64c

pdurbin requested changes Apr 14, 2022

View reviewed changes

doc/sphinx-guides/source/admin/integrations.rst Outdated Show resolved Hide resolved

doc/sphinx-guides/source/developers/workflows.rst Outdated Show resolved Hide resolved

pdurbin assigned qqmyers Apr 14, 2022

qqmyers and others added 3 commits April 14, 2022 10:45

Update doc/sphinx-guides/source/developers/workflows.rst

109a4a1

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

typo - remove :

f6cea7c

Merge branch 'TDL/7493-make_BagGenerate_threads_configurable_only' of h…

32f3a58

…ttps://github.com/TexasDigitalLibrary/dataverse.git into TDL/7493-make_BagGenerate_threads_configurable_only

pdurbin mentioned this pull request Apr 14, 2022

fix make html errors #7493 #8602 TexasDigitalLibrary/dataverse#2

Closed

pdurbin approved these changes Apr 14, 2022

View reviewed changes

pdurbin unassigned pdurbin and qqmyers Apr 14, 2022

kcondon self-assigned this Apr 14, 2022

kcondon merged commit bb598fc into IQSS:develop Apr 14, 2022

pdurbin added this to the 5.11 milestone May 4, 2022

qqmyers mentioned this pull request Jun 3, 2022

add 5.11 release notes #8767

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TDL/7493 Make BagGenerator threads configurable #8606

TDL/7493 Make BagGenerator threads configurable #8606

qqmyers commented Apr 12, 2022

coveralls commented Apr 12, 2022 •

edited

Loading

pdurbin left a comment

pdurbin Apr 13, 2022

pdurbin Apr 13, 2022

qqmyers Apr 13, 2022

pdurbin Apr 14, 2022

pdurbin Apr 14, 2022

pdurbin Apr 14, 2022

pdurbin left a comment

pdurbin left a comment

		@@ -180,7 +180,7 @@ archiver

		A step that sends an archival copy of a Dataset Version to a configured archiver, e.g. the DuraCloud interface of Chronopolis. See the `DuraCloud/Chronopolis Integration documentation <http://guides.dataverse.org/en/latest/admin/integrations.html#id15>`_ for further detail.


		A Dataverse installation can be configured to submit a copy of published Datasets, packaged as `Research Data Alliance conformant <https://www.rd-alliance.org/system/files/Research%20Data%20Repository%20Interoperability%20WG%20-%20Final%20Recommendations_reviewed_0.pdf>`_ zipped `BagIt <https://tools.ietf.org/html/draft-kunze-bagit-17>`_ bags to the `Chronopolis <https://libraries.ucsd.edu/chronopolis/>`_ via `DuraCloud <https://duraspace.org/duracloud/>`_
		A Dataverse installation can be configured to submit a copy of published Datasets, packaged as `Research Data Alliance conformant <https://www.rd-alliance.org/system/files/Research%20Data%20Repository%20Interoperability%20WG%20-%20Final%20Recommendations_reviewed_0.pdf>`_ zipped `BagIt <https://tools.ietf.org/html/draft-kunze-bagit-17>`_ bags to the `Chronopolis <https://libraries.ucsd.edu/chronopolis/>`_ via `DuraCloud <https://duraspace.org/duracloud/>`_, to a local file system, or to `Google Cloud Storage<https://cloud.google.com/storage>`_.

TDL/7493 Make BagGenerator threads configurable #8606

TDL/7493 Make BagGenerator threads configurable #8606

Conversation

qqmyers commented Apr 12, 2022

coveralls commented Apr 12, 2022 • edited Loading

pdurbin left a comment

Choose a reason for hiding this comment

pdurbin Apr 13, 2022

Choose a reason for hiding this comment

pdurbin Apr 13, 2022

Choose a reason for hiding this comment

qqmyers Apr 13, 2022

Choose a reason for hiding this comment

pdurbin Apr 14, 2022

Choose a reason for hiding this comment

pdurbin Apr 14, 2022

Choose a reason for hiding this comment

pdurbin Apr 14, 2022

Choose a reason for hiding this comment

pdurbin left a comment

Choose a reason for hiding this comment

pdurbin left a comment

Choose a reason for hiding this comment

coveralls commented Apr 12, 2022 •

edited

Loading