-
Notifications
You must be signed in to change notification settings - Fork 113
add ccs support for 8.19 and 9.1 #2129
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
base: main
Are you sure you want to change the base?
Conversation
🔍 Preview links for changed docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for handling this!
We need to make some adjustments, let me know if you believe I've missed anything!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for jumping on this @ketkee-aryamane! First off, the table is awful to work with 🤣 and there's additional complexity because 8.18
wasn't 8.last
.
Looking at the new table, it does look like we've introduced an error here: it now states that a local 9.0 is not compatible with a remote 8.18. This contradicts the explicit example text "For example, a local 9.0 cluster can search a remote 8.18 or any remote 9.x cluster. "
… into 313-add-ccs-support
:::{note} | ||
As with version 8.19, cross-cluster searches are supported between a local 8.18 cluster and a remote 9.0 cluster, in both directions. This is possible because version 9.0 was released before 8.19. For more information on the order of releases, see [Elastic Search Releases.](https://github.com/elastic/elasticsearch/releases) | ||
::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the note! It's a good way to explain the special case of the table that doesn't really follow the default rules described at the beginning of the section.
Have you checked how it looks adding also something like (*)
in the items that are part of this exception?
What I don't like much on the proposed text is the link we are using, but not sure what to use.
I was thinking of this link because it talks about a similar topic, but that content needs to be added to the new docs (we lost it during the docs migration, and the link I'm showing you is for the legacy asciidoc docs). I'm planning to work on that soon.
@shainaraskas , could you share your thoughts here?
This note clarifies why certain items of the big table do not exactly follow the general rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried the asterisk mark, but it looks a little out of place Edu :). I'll have to search elsewhere in the repo of how I can include that next to the check marks and make it look seamless with the UI.
Also, wanted to understand why it would not be preferable to include our Git links.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thoughts @eedugon , do we really need the note? We're anyway offering more compatibility, not less. So just check all correct boxes and get done? :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the links to the elasticsearch Github repo directly there isn't any strong reason. It's just that I believe we don't have many links like those in the docs, and we try to link to other docs or other type of pages if possible.
For example for the releases information
, besides the official Github page that you have shared we have https://www.elastic.co/downloads/past-releases#elasticsearch (which by the way it's not the nicest page from UX point of view :) ).
So, in this case, considering we have an alternative page about releases, maybe it would be good to use that. But after thinking a bit more about this exact case, I think your link is perfectly valid too.
Anyway, if I'm able to include the out of order releases
information in our documents soon (in a different PR), we can use that link in this PR, and from the out of order releases
information I'd like to link to the releases information.
For the moment use whatever you prefer! :)
Oh, and remember to not merge this PR until 9.1 / 8.19 are released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thoughts @eedugon , do we really need the note? We're anyway offering more compatibility, not less. > So just check all correct boxes and get done? :))
In my opinion, and this is only my opinion, if a reader follows the statements about compatibility and then analyze a bit the table, they could believe the table is buggy and cause confusion like is that really true?
, which part of the doc is wrong?
, are the statements about compatibility incorrect or is the table incorrect?
).
Because of that I think clarifying such an exception against the statements is good. Or simply make a note saying that the table might have extra compatible items due to special releases that were released "out of order" (with a link to the out of order info doc I have to work with in another PR :) ).
But.... do we really need the note? Maybe not :-D But I still belive we do, or at least something saying that the rules
have certain exceptions due to out order releases
.
Another option would be keeping the table totally standard, following the rules, and not showing this extra compatibility
. We could check that with developers as soon as we are done with this PR preparations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the current link isn't very helpful, and I think we can probably capture the important exceptional callouts in prose (or a note) without needing to link anywhere. If concerned about exactly how to message the exceptions, be sure to reach out in #cross-cluster-search
for product and SME input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to work before 9.1 in a PR that explains the out of order
releases (like this), and my plan is to include a statement that would indirectly explain this example (as I'll mention specifically 8.18 and 9.0).
After that is added, we could simplify this a bit and link to that doc for extra explanations.
What do you think @leemthompo and @ketkee-aryamane ? We can wait a bit to see how that looks.
The main objective (IMO) is to ensure the reader understands that certain compatible cells
don't match the initial statements because exceptional out of order releases require that compatibility.
If a reader doesn't fully understand the why
but at least the doc clearly says that the unexpected
compatible items have a reason at least that shouldn't cause confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure let's see, I assigned this ticket to work on a pretty small, but non-trivial piece of explanation, that is a pretty good microcosm of the complexities of working on Elastic docs. Ketkee needn't be in a rush to finish this, but should lean into the learnings and make sure the writing is clear as a first non-negotiable.
If a reader doesn't fully understand the why but at least the doc clearly says that the unexpected compatible items have a reason at least that shouldn't cause confusion.
Yes the clear statement is what we should aim for, and if it's sufficiently clear it will clarify why 8.18 was exceptional and ensure the description and the table don't contradict each other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's still a pending change that I've added in a comment.
About the note I think we should update the link and not linking to the github page of Elasticsearch from our official docs. If feels really weird. But I'm not sure where to link at the moment.
[Elastic Search versions](https://www.elastic.co/downloads/past-releases#elasticsearch) were not released in a sequential order. Hence, additional compatibility conditions may apply. | ||
As shown in the following table, cross-cluster searches are supported between a local 8.18 cluster and a remote 9.0 cluster, in both directions. This is same as version 8.19 and is possible because version 9.0 was released before 8.19. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Elastic Search versions](https://www.elastic.co/downloads/past-releases#elasticsearch) were not released in a sequential order. Hence, additional compatibility conditions may apply. | |
As shown in the following table, cross-cluster searches are supported between a local 8.18 cluster and a remote 9.0 cluster, in both directions. This is same as version 8.19 and is possible because version 9.0 was released before 8.19. | |
Version 8.19 is the last minor version of the 8.x series. Previously, the last minor version of a major series was released simultaneously with the first version of the next major series. However, 8.18 was released simultaneously with 9.0, creating bidirectional compatibility between these specific versions. As shown in the compatibility table, 8.18 can search 9.0 clusters, but unlike 8.19, it cannot search versions 9.1+. |
I suggested this edit because the wording wasn't super precise for me.
- For example,
Hence, additional compatibility conditions may apply
is too vague. - I think the reference to non-sequential ordering is confusing if we don't explain the specifics here.
- Also, although, I ❤️ that linked page I'm not sure if it fits here. As mentioned earlier, the table itself should be accurate—so you don't need to reference that past releases page to determine compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with @leemthompo's suggestion, although the previously part
might not add much benefit. I'd keep the rest.
My idea was just to say something due to how certain versions are released out of order, the table shows extra compatibility cases not covered in the previous statements
, but if you prefer to show the specific example that's perfect too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
although the
previously
part might not add much benefit. I'd keep the rest.
Could lose that sentence for sure 👍
My idea was just to say something due to how certain versions are released out of order, the table shows extra compatibility cases not covered in the previous statements
Right— I think being specific might be more clear here :)
But I think once we've agreed should bring in PMs to validate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is too much info for a note & sounds confusing for me to read. Want to keep it concise. I can lose the vague statement and include the 8.18 thing as an example for the additional compatibility to make it clearer.
To avoid that explanation, I provided the link so that the reader could refer to release dates, if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is too much info for a note & sounds confusing for me to read. Want to keep it concise.
We are not in a rush here, and if a note isn't the right format for delivering this information, this could be a heading about the specificities of 8.18 and 8.19 to help the user understand the context. For me explanation > link to a list of release dates (which don't explain CCS compatibility).
Take your time with this, we have until 9.1 releases to make this page as useful as possible :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note or not, I'm not sure if an explanation is needed and I'd prefer the link. I'll rephrase this a little bit and then prefer the devs to comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, you know better than me ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not saying I know better. This is based on what I’ve seen/read before from the devs perspective.
I'll also wait on @eedugon's doc and if that is clear enough, can link it from here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the noise, I think there's a few too many cooks in the kitchen here, and there was a mix of copyediting + disagreements about how to present information. But as I said when I opened the issue we shouldn't have to guess too much here, we should ask the CCS experts how they want to communicate the specificities of 8.18/8.19. :)
Co-authored-by: Liam Thompson <leemthompo@gmail.com>
DO NOT MERGE THIS PR UNTIL 9.1 / 8.19 RELEASE DAY
Linked to https://github.com/elastic/developer-docs-team/issues/313
Added versions 8.19 and 9.1 to the table listing support for cross cluster search. The following screenshot gives a preview of the change.
Added the 'applies_to' tag to signify GA for 9.1.