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

ISPN-14130 Add the memory option in the data Distribution chart #266

Merged

Conversation

dpanshug
Copy link
Collaborator

@dpanshug dpanshug requested a review from karesti October 10, 2022 11:46
@dpanshug dpanshug self-assigned this Oct 10, 2022
@dpanshug dpanshug force-pushed the ISPN-14130-memory-data-distribution branch 2 times, most recently from 005d284 to 2df7de9 Compare October 13, 2022 11:37
Copy link
Collaborator

@karesti karesti left a comment

Choose a reason for hiding this comment

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

there is something with the formatter of the code that is not ok.
could you please run prettier? npm run format

src/app/Caches/DataDistributionChart.tsx Outdated Show resolved Hide resolved
src/app/Caches/DataDistributionChart.tsx Outdated Show resolved Hide resolved
src/app/Caches/DataDistributionChart.tsx Outdated Show resolved Hide resolved
src/app/Caches/DataDistributionChart.tsx Outdated Show resolved Hide resolved
@dpanshug dpanshug force-pushed the ISPN-14130-memory-data-distribution branch 3 times, most recently from f27871f to 9e1dd42 Compare October 17, 2022 10:42
@dpanshug
Copy link
Collaborator Author

@karesti code might be changing because of prettier formatting, will create another PR for improving it in other files also.

@dpanshug dpanshug removed the preview label Oct 17, 2022
@dpanshug dpanshug force-pushed the ISPN-14130-memory-data-distribution branch from 9e1dd42 to 03fbe46 Compare October 17, 2022 15:17
@karesti
Copy link
Collaborator

karesti commented Oct 20, 2022

@dpanshug needs rebase

@karesti
Copy link
Collaborator

karesti commented Oct 24, 2022

@dpanshug did you do any changes on this PR ?

@dpanshug dpanshug force-pushed the ISPN-14130-memory-data-distribution branch from 57e0143 to f0470c6 Compare October 26, 2022 08:18
@dpanshug
Copy link
Collaborator Author

@dpanshug did you do any changes on this PR ?

I've pushed the changes now.

@dpanshug dpanshug requested a review from karesti October 26, 2022 08:19
Copy link
Collaborator

@karesti karesti left a comment

Choose a reason for hiding this comment

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

I'm testing the feature where the endpoint answers

[
  {
    "node_name": "3803a7d55fbe-10729",
    "node_addresses": [
      "172.17.0.2:7800"
    ],
    "memory_entries": 1,
    "total_entries": 1,
    "memory_used": 104
  }
]

And nothing is displayed. I added one entry to a cache with this config:

Cache config

{
  "vvbvbvvbb": {
    "distributed-cache": {
      "owners": "1",
      "mode": "SYNC",
      "statistics": true,
      "encoding": {
        "key": {
          "media-type": "application/x-protostream"
        },
        "value": {
          "media-type": "application/x-protostream"
        }
      },
      "locking": {
        "isolation": "REPEATABLE_READ"
      },
      "memory": {
        "max-size": "56MB",
        "when-full": "REMOVE"
      }
    }
  }
}

@dpanshug
Copy link
Collaborator Author

@karesti I've updated the validation for displaying memory_used, now memory_used will work only when max-size is configured.

@dpanshug dpanshug requested a review from karesti October 28, 2022 08:20
Copy link
Collaborator

@karesti karesti left a comment

Choose a reason for hiding this comment

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

rebase please!

@dpanshug dpanshug force-pushed the ISPN-14130-memory-data-distribution branch from e51365b to fe7da6b Compare October 31, 2022 04:54
@dpanshug dpanshug requested a review from karesti October 31, 2022 06:14
@karesti karesti merged commit e9db46b into infinispan:main Nov 2, 2022
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.

None yet

2 participants