Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autogenerate java docs #29

Closed
wants to merge 54 commits into from
Closed

Conversation

dgarcia360
Copy link

@dgarcia360 dgarcia360 commented Aug 7, 2020

@lauranovich @tzach Before merging this PR, please merge #30

  • Added Sphinx support: fixed links, fixed warnings, changed the folder structure, and added build scripts.
  • Added sphinx-scylladb-theme.
  • Auto-build Sphinx docs and doxygen using GitHub Actions.
  • Added multiversion support. Now only builds docs for the latest branch, but conf.py script can be edited to build other documentation versions from this commit. See Multiversion support.

Note: Versions tagged previously to this PR do not have a Sphinx project installed, so it won't be possible to autogenerate docs for those.

haaawk and others added 30 commits August 7, 2019 14:47
This is required to allow new code owned by ScyllaDB.

Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Part of Connection initialization will be obtaining
info about shards in the host so Connection needs
a reference to the host to be able to set up this info.

Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
This field will track the details of shards available at the host.
It will be set during connection initialization.
Multiple such initializations can occure at the same time but
each should be setting the same number of shards.

Signed-off-by: Piotr Jastrzebski <[email protected]>
This field will be set during initialization and will
describe the shard in the host that a connection is associated to.

Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
This field will be used for paging queries to fetch all
pages from the same host if possible.

Signed-off-by: Piotr Jastrzebski <[email protected]>
This policy will try to fetch all pages from
the same host if possible.

Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
HostConnectionPool.cleanupIdleConnections is called from
a recurring task. It can race with the initialization
of the pool leading to NPE and other concurrency problems.

Signed-off-by: Piotr Jastrzebski <[email protected]>
By default the driver has the ability to coalesce schem refresh queries
to try and reduce the number of queries sent. In case a view and a table
refresh is needed a refresh for the keyspace would be generated.

While in most cases thisoptimization is helpfull - in case there are
100s of tables in a single keyspace this will cause fetching all
keyspace information which will cause extra load on the system (in
scylla's case a single shard).

Add the ability to disable this by reusing setting of
maxPendingRefreshSchmaRequests value. If its 1 disable coalescing.

Signed-off-by: Shlomi Livne <[email protected]>
Signed-off-by: Piotr Jastrzebski <[email protected]>
Make sure groupId is set everywhere to com.scylladb and
artifactId has scylla- prefix instead of cassandra- prefix.

Signed-off-by: Piotr Jastrzebski <[email protected]>
Those metrics expose the information the driver has
about number of shards each node has.

New metric is called shard-awareness-info and its
value is a Map<com.datastax.driver.core.Host, Integer>.

For each host (which represents a node in a cluster) it presents
either the number of shards the host has or null if the node
does not provide sharding information.

Signed-off-by: Piotr Jastrzebski <[email protected]>
Previously the pool wasn't available for users before we have created
requested number of connections to each shard. This was realized by
not making a future returned from initAsync ready.

Some users were reporting the initialization taking
too long and timing out.

This commit changes the way initAsync works. From now on:
1. initAsync will try to open 2 * |number of shards| connections
2. Then it will close connections that are excessive for each shard
3. For shards that didn't get enough connections, connection tasks
   will be scheduled to create missing connections.

Also we are limiting the number of connections each ConnectionTask
holds to no more than the number of shards.

Signed-off-by: Piotr Jastrzebski <[email protected]>
if connections for a target shard are still being
initialized.

Signed-off-by: Piotr Jastrzebski <[email protected]>
Previously each ConnectionTask was keeping its own excessive connections
(connections that are not to the shard it wants).

This patch changes the behaviour so that all ConnectionTasks
share excessive connections. This way we not only better limit
number of excessive connections but also will quicker find
the right connection for each task (some other task may open
a connection for it).

Signed-off-by: Piotr Jastrzebski <[email protected]>
@dgarcia360 dgarcia360 requested a review from tzach August 28, 2020 14:22
@dgarcia360
Copy link
Author

@tzach I ended overriding the markdown parser to have more flexible relative links: https://github.com/scylladb/java-driver/pull/29/files#diff-1ec61223727a10e5e4c74c68336809fcR80-R101

If you like the solution, we could move part of the logic tot he sphinx_scylladb_theme to make this functionality available to all the documentation projects.

@tzach
Copy link

tzach commented Sep 14, 2020

@tzach I ended overriding the markdown parser to have more flexible relative links: https://github.com/scylladb/java-driver/pull/29/files#diff-1ec61223727a10e5e4c74c68336809fcR80-R101

Nice. Can you trying pushing this to upstream, so we do not have to manage this code?

@tzach
Copy link

tzach commented Sep 14, 2020

@dgarcia360 can we automate the READM -> Index rename, so we do not have to do it for each project and sync with the driver upstram?

@dgarcia360
Copy link
Author

dgarcia360 commented Sep 14, 2020

@tzach Edited the PR to keep all the docs files in their original folders by adding symbolic links and removed the index references from all the documents to make it easier once we rename the files from index to README.

can we automate the READM -> Index rename, so we do not have to do it for each project and sync with the driver upstram?

I'm on it. Already managed to achieve this for the make preview and make dirhml command, but the solution is not working with make multiversion because it's not possible to run custom commands between version builds.

I have already raised the issue here sphinx-contrib/multiversion#45 and submitted a possible solution Holzhaus/sphinx-multiversion#46. Let's wait some days hoping this is merged, if not we'll evaluate other alternatives to rename the files.

Nice. Can you trying pushing this to upstream, so we do not have to manage this code?

The upstream repo datastax/java-driver doesn't have a conf.py file. The code won't be accepted in the original recommonmark extension because is removing the functionality that validates if the relative link file exists to suit the use case. What we can do is moving part of the logic to sphinx_scylladb_theme to make this functionality available to all the documentation projects once we merge this PR.

@dgarcia360
Copy link
Author

@tzach @lauranovich The PR is ready to be reviewed. The latest commits automate the README -> Index using the forked repository https://github.com/dgarcia360/sphinx-multiversion. Additionally, I have modified the redirections scripts to support multi-version. Some of these changes will be added to the main theme once this PR is accepted.

@lauranovich lauranovich mentioned this pull request Sep 21, 2020
@lauranovich
Copy link

Doc renders nicely and theme is applied.
the only thing I see which needs to be fixed (in a second PR)
is that the docs are branded Datastax.
It needs to be Scylla.

I'll handle this in a separate PR. - @tzach please take a look

@haaawk
Copy link

haaawk commented Sep 25, 2020

Something's wrong with this PR. Why does it have 54 commits - some of them already existing in the repository?

@dgarcia360
Copy link
Author

@haaawk The PR was issued when scylla-3.7.1 was tagged as the main branch (Nov 2019). Then, when the lastest branch "latest" was created, I had to update the PR to reflect the newer commits added (2020).

From the files changes, nothing other than the java docs generation has been added. You could either squash and merge this PR to make it one commit, or I could issue a separate PR with just one commit. Please, let me know which option do you prefer.

@haaawk
Copy link

haaawk commented Sep 28, 2020

A separate PR with just one commit would be perfect, @dgarcia360. Thank you!

@dgarcia360
Copy link
Author

Closed in favor of #38

@dgarcia360 dgarcia360 closed this Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants