Skip to content

Commit

Permalink
Remove inaccurate pre-check in shrink index page (#558)
Browse files Browse the repository at this point in the history 
Change some wording in shrink index page

Signed-off-by: gaobinlong <[email protected]>
  • Loading branch information
@gaobinlong
gaobinlong committed Jan 12, 2023
1 parent aec8569 commit 5164cba
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 51 deletions.
20 changes: 4 additions & 16 deletions public/pages/ShrinkIndex/container/ShrinkIndex/ShrinkIndex.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ describe("<Shrink index /> spec", () => {
const { queryByText, getByTestId } = renderWithRouter([`${ROUTES.SHRINK_INDEX}?source=test4`]);

await waitFor(() => {
expect(queryByText("The source index must be in Green health status.")).not.toBeNull();
expect(queryByText("The source index must be in green health status.")).not.toBeNull();
expect(getByTestId("shrinkIndexConfirmButton")).toHaveAttribute("disabled");
});
});
Expand Down Expand Up @@ -488,7 +488,7 @@ describe("<Shrink index /> spec", () => {
getByText("Source index details");
});

expect(queryByText("We recommend shrinking index with a Green health status.")).not.toBeNull();
expect(queryByText("We recommend shrinking index with a green health status.")).not.toBeNull();
});

it("shows warning when source index is set to read-only", async () => {
Expand Down Expand Up @@ -563,17 +563,6 @@ describe("<Shrink index /> spec", () => {
});
});

it("shows warning when source index's has no index.routing.allocation.require._* setting", async () => {
mockApi();
const { getByText, queryByText } = renderWithRouter([`${ROUTES.SHRINK_INDEX}?source=test6`]);

await waitFor(() => {
getByText("Source index details");
});

expect(queryByText("A copy of every shard must reside on the same node.")).not.toBeNull();
});

it("no warning when source index is ready", async () => {
mockApi();
const { getByText, queryByText, getByTestId } = renderWithRouter([`${ROUTES.SHRINK_INDEX}?source=test3`]);
Expand All @@ -582,13 +571,12 @@ describe("<Shrink index /> spec", () => {
getByText("Configure target index");
});

expect(queryByText("The source index must be in Green health status.")).toBeNull();
expect(queryByText("The source index must be in green health status.")).toBeNull();
expect(queryByText("Cannot shrink source index with only one primary shard.")).toBeNull();
expect(queryByText("The source index must block write operations before shrinking.")).toBeNull();
expect(queryByText("The source index must be open.")).toBeNull();
expect(queryByText("We recommend shrinking index with a Green health status.")).toBeNull();
expect(queryByText("We recommend shrinking index with a green health status.")).toBeNull();
expect(queryByText("Index setting [index.blocks.read_only] is [true].")).toBeNull();
expect(queryByText("A copy of every shard must reside on the same node.")).toBeNull();

userEvent.type(getByTestId("targetIndexNameInput"), "test3_shrunken");

Expand Down
42 changes: 7 additions & 35 deletions public/pages/ShrinkIndex/container/ShrinkIndex/ShrinkIndex.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,10 @@ export default class ShrinkIndex extends Component<ShrinkIndexProps, ShrinkIndex
disableShrinkButton = true;
sourceIndexNotReadyToShrinkReasons.push(
<>
<EuiCallOut title="The source index must be in Green health status." color="danger" iconType="alert">
<EuiCallOut title="The source index must be in green health status." color="danger" iconType="alert">
<p>
The index is in Red health status and may be running operations in the background. We recommend to wait until the index
becomes Green to continue shrinking.
The index is in red health status and may be running operations in the background. We recommend to wait until the index
becomes green to continue shrinking.
</p>
</EuiCallOut>
<EuiSpacer />
Expand Down Expand Up @@ -367,7 +367,7 @@ export default class ShrinkIndex extends Component<ShrinkIndexProps, ShrinkIndex
<EuiCallOut title="The source index must be open." color="danger" iconType="alert">
<p>
You must first open the index before shrinking it. Depending on the size of the source index, this may take additional time
to complete. The index will be in the Red state while the index is opening.
to complete. The index will be in red health status while the index is opening.
</p>
<EuiSpacer />
<EuiButton
Expand All @@ -393,10 +393,10 @@ export default class ShrinkIndex extends Component<ShrinkIndexProps, ShrinkIndex
if (sourceIndex.health === "yellow") {
sourceIndexNotReadyToShrinkReasons.push(
<>
<EuiCallOut title="We recommend shrinking index with a Green health status." color="warning" iconType="help">
<EuiCallOut title="We recommend shrinking index with a green health status." color="warning" iconType="help">
<p>
The source index is in Yellow health status. To prevent issues with initializing the new shrunken index, we recommend
shrinking an index with a Green health status.
The source index is in yellow health status. To prevent issues with initializing the new shrunken index, we recommend
shrinking an index with a green health status.
</p>
</EuiCallOut>
<EuiSpacer />
Expand All @@ -421,34 +421,6 @@ export default class ShrinkIndex extends Component<ShrinkIndexProps, ShrinkIndex
</>
);
}

// This check may not be accurate in the following cases:
// 1. the cluster only has one node, so the source index's primary shards are allocated to the same node.
// 2. the primary shards of the source index are just allocated to the same node, not manually.
// 3. the user set `index.routing.rebalance.enable` to `none` and then manually move each shard's copy to one node.
// In the above cases, the source index does not have a `index.routing.allocation.require._*` setting which can
// rellocate one copy of every shard to one node, but it can also execute shrinking successfully if other conditions are met.
// But in most cases, source index always have many shards distributed on different node,
// so index.routing.allocation.require._*` setting is required.
// In above, we just show a warning in the page, it does not affect any button or form.
const settings = get(sourceIndexSettings, [sourceIndex.index, "settings"]);
let shardsAllocatedToOneNode = false;
for (let settingKey in settings) {
if (settingKey.startsWith(INDEX_ROUTING_ALLOCATION_SETTING)) {
shardsAllocatedToOneNode = true;
break;
}
}
if (!shardsAllocatedToOneNode) {
sourceIndexNotReadyToShrinkReasons.push(
<>
<EuiCallOut title="A copy of every shard must reside on the same node." color="warning" iconType="help">
<p>For clusters with more than one node, you must allocate a copy of every shard of the source index to the same node.</p>
</EuiCallOut>
<EuiSpacer />
</>
);
}
}
}

Expand Down

0 comments on commit 5164cba

Please sign in to comment.