Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

[DO NOT MERGE] EOS-26465: DTM support - PUT KV in non-batch mode #1653

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[DO NOT MERGE] EOS-26465: DTM support - PUT KV in non-batch mode #1653

wants to merge 2 commits into from

Conversation

DPG17
Copy link
Contributor

@DPG17 DPG17 commented Dec 9, 2021

Signed-off-by: Dattaprasad Govekar [email protected]

Problem Statement

  • Problem statement

Design

  • For Bug, Describe the fix here.
  • For Feature, Post the link for design

Coding

Checklist for Author

  • Coding conventions are followed and code is consistent

Testing

Checklist for Author

  • Unit and System Tests are added
  • Test Cases cover Happy Path, Non-Happy Path and Scalability
  • Testing was performed with RPM

Impact Analysis

Checklist for Author/Reviewer/GateKeeper

  • Interface change (if any) are documented
  • Side effects on other features (deployment/upgrade)
  • Dependencies on other component(s)

Review Checklist

Checklist for Author

  • JIRA number/GitHub Issue added to PR
  • PR is self reviewed
  • Jira and state/status is updated and JIRA is updated with PR link
  • Check if the description is clear and explained

Documentation

Checklist for Author

  • Changes done to WIKI / Confluence page / Quick Start Guide

@cla-bot cla-bot bot added the cla-signed label Dec 9, 2021
@DPG17 DPG17 self-assigned this Dec 13, 2021
@DPG17 DPG17 changed the title EOS-26465: PUT KV in non-batch mode EOS-26465: DTM support: PUT KV in non-batch mode Dec 14, 2021
@DPG17 DPG17 changed the title EOS-26465: DTM support: PUT KV in non-batch mode EOS-26465: DTM support - PUT KV in non-batch mode Dec 14, 2021
t7ko-seagate
t7ko-seagate approved these changes Dec 14, 2021
View reviewed changes
server/s3_motr_kvs_writer.cc Outdated
parallel_writer_contexts.clear();
const int parallel_kvs =
(kv_list.size() <= max_parallel_kv) ? kv_list.size() : max_parallel_kv;
// kvop_keys_in_flight = parallel_kvs;
Copy link
Contributor

Choose a reason for hiding this comment

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

stale comment - remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Removed from other place as well.

knrajnambiar76
knrajnambiar76 approved these changes Dec 14, 2021
View reviewed changes
server/s3_motr_kvs_writer.cc Outdated
void S3MotrKVSWriter::parallel_put_kv_failed(unsigned int processed_count) {
s3_log(S3_LOG_DEBUG, request_id, "%s Entry\n", __func__);
next_key_offset += processed_count;
atleat_one_failed = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

typo atleast_...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@stale
Copy link

stale bot commented Dec 18, 2021

This issue/pull request has been marked as needs attention as it has been left pending without new activity for 4 days. Tagging @nileshgovande @bkirunge7 @knrajnambiar76 @t7ko-seagate for appropriate assignment. Sorry for the delay & Thank you for contributing to CORTX. We will get back to you as soon as possible.

@DPG17
EOS-26465: Non-batch put kv
e8e225d 
Signed-off-by: Dattaprasad Govekar <[email protected]>
@stale stale bot removed the needs-attention label Dec 20, 2021
github-actions[bot]
github-actions bot reviewed Dec 20, 2021
View reviewed changes
scripts/env/kubernetes/README.md Outdated
RAM.
* Open terminal.
* Switch to a `root` user.
* Execute execute commands listed below.
Copy link

Choose a reason for hiding this comment

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

⚠️ [alex] reported by reviewdog 🐶
Be careful with Execute, it’s profane in some cases execute retext-profanities

scripts/env/kubernetes/README.md Outdated
RAM.
* Open terminal.
* Switch to a `root` user.
* Execute execute commands listed below.
Copy link

Choose a reason for hiding this comment

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

⚠️ [alex] reported by reviewdog 🐶
Be careful with execute, it’s profane in some cases execute retext-profanities

scripts/env/kubernetes/README.md Outdated
* Open terminal.
* Switch to a `root` user.
* Execute execute commands listed below.
* Simply copy the entire commands list from each box, and paste into the
Copy link

Choose a reason for hiding this comment

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

⚠️ [alex] reported by reviewdog 🐶
Simply may be insensitive, try not to use it simple retext-equality

@DPG17 DPG17 changed the base branch from main to kubernetes December 20, 2021 07:43
@DPG17
Copy link
Contributor Author

DPG17 commented Dec 21, 2021

Pre-merge test is success. See here http://eos-jenkins.colo.seagate.com/job/S3server/job/S3Server-PreMerge-Test/838/

@t7ko-seagate
Copy link
Contributor

Let us hold this merge until final clarity comes from Motr team -- as they were going to make similar change from Motr side.

@t7ko-seagate t7ko-seagate changed the title EOS-26465: DTM support - PUT KV in non-batch mode [DO NOT MERGE] EOS-26465: DTM support - PUT KV in non-batch mode Dec 24, 2021
@t7ko-seagate
Copy link
Contributor

This PR is put on pause, because Motr team is trying to implement changes on their side.

@stale
Copy link

stale bot commented Jan 2, 2022

This issue/pull request has been marked as needs attention as it has been left pending without new activity for 4 days. Tagging @nileshgovande @bkirunge7 @knrajnambiar76 @t7ko-seagate for appropriate assignment. Sorry for the delay & Thank you for contributing to CORTX. We will get back to you as soon as possible.

@stale stale bot added the needs-attention label Jan 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@knrajnambiar76 knrajnambiar76 knrajnambiar76 approved these changes

@t7ko-seagate t7ko-seagate t7ko-seagate approved these changes

@bkirunge7 bkirunge7 Awaiting requested review from bkirunge7

@nileshgovande nileshgovande Awaiting requested review from nileshgovande

At least 2 approving reviews are required to merge this pull request.

Assignees

@DPG17 DPG17

Labels
cla-signed Maverick needs-attention
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DPG17 @t7ko-seagate @knrajnambiar76