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

JBOD support documentation #1280

Merged
merged 9 commits into from Feb 19, 2019
Merged

JBOD support documentation #1280

merged 9 commits into from Feb 19, 2019

Conversation

ppatierno
Copy link
Member

Type of change

  • Documentation

Description

This PR adds documentation for the JBOD support.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Update/write design documentation in ./design
  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md

@ppatierno
Copy link
Member Author

I am still working on it but having a WIP is better for starting to fix the comments.

@ppatierno ppatierno changed the title WIP JBOD support documentation JBOD support documentation Feb 1, 2019
@ppatierno ppatierno added this to the 0.11.0 milestone Feb 1, 2019
Copy link
Member

@tombentley tombentley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits, but technically it's fine.

documentation/book/assembly-storage.adoc Outdated Show resolved Hide resolved
documentation/book/ref-storage-jbod.adoc Outdated Show resolved Hide resolved
documentation/book/ref-storage-jbod.adoc Outdated Show resolved Hide resolved
documentation/book/ref-storage-jbod.adoc Outdated Show resolved Hide resolved
documentation/book/ref-storage-jbod.adoc Outdated Show resolved Hide resolved
# ...
----

It is not possible to change the `id` property for a volume.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement seems a little ambiguous. Did you mean the following: "It is not possible to change the id property for a volume after it is provisioned"?

Do the id properties need to be set to 0, 1, 2, 3, 4 etc. in ascending order?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, to both the questions :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I just need to update that sentence before the PR is merged.

@d-laing
Copy link
Member

d-laing commented Feb 18, 2019

Peer reviewed the content by checking out the remote branch. @ppatierno - please can you check it over, thanks.

Storage can be configured using the `storage` property in following resources:
NOTE: JBOD storage is only supported for Kafka, not for Zookeeper.

When configuring a `kafka` resource, you can specify the type of storage used by the Kafka broker and its corresponding Zookeeper node. You configure the storage type using the `storage` property in the following resources:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kafka has to be with capital K it's about the kind of the resource, it's not the property spec.kafka


The JBOD storage allows you to provide more volumes to the Kafka brokers to store data.
As well as providing more storage, using multiple disks can improve performance.
You can configure {ProductName} to use Just a Bunch of Disks (JBOD), a data storage configuration in which multiple disks are combined to create a single logical disk, or volume. JBOD is one approach to providing increased data storage for Kafka brokers. It can also improve performance.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the other commit, you used the other way around ... JBOD (Just a Bunch of Disks), isn't it better to be consistent?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Be Consistent! :) But Just a Bunch of Disks (JBOD) is the order it should be displayed in. So I'd change the other section referenced.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ncbaratta tbh I think that all developers/engineers refer to it as JBOD more than "Just a Bunch of Disks". I would refer it as JBOD and the having the acronym explained in parenthesis ... so quite the opposite you said :P

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If most developers just know what it is then I'd remove the spelled out name. If people don't refer to it that way, then we don't have to define it at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good discussion here :). I think I'll remove the spelled out name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can remove the spelled out name here but I would still have it in the assembly-storage.adoc file.

storage:
type: jbod
# ...
----
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason for having this "empty" example just setting the storage jbod type without volumes, because it cannot work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see your point. I just wanted to call out the relevant type. I'll remove it.

@@ -34,10 +40,13 @@ storage:
# ...
----

It is not possible to change the `id` property for a volume.
It is not possible to add or remove volumes from a JBOD storage configuration.
Do not change the `id` property of a JBOD volume.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh the statement I used sounded clearer than this one. We should say that after volumes creation with some ids, they cannot be changed anymore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I will update.


`data-_id_-_cluster-name_-kafka-_idx_`::
Persistent Volume Claim for the volume `_id_` used for storing data for the Kafka broker pod `_idx_`.
`data-_id_-cluster-name-kafka-_idx_`::
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the "cluster-name" part is variable as id and idx should be used "_" as I did ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry about that. I will update.

@d-laing
Copy link
Member

d-laing commented Feb 18, 2019

@ppatierno - I've applied the changes you suggested to the reviewed content. Apart from the failing Travis CI build, I think this is good to merge.

@ppatierno ppatierno merged commit 075c103 into master Feb 19, 2019
@ppatierno ppatierno deleted the jbod-doc branch February 19, 2019 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants