[2.0] Deprecate the "Master" nomenclature in nodejs client #222

ananzh · 2022-04-21T07:03:52Z

Description

1.Deprecate setting cluster.initial_master_nodes and introduce
the alternative setting cluster.initial_cluster_manager_nodes.
2.Use a new node role cluster_manager that has the same functionality
with master in the node setting node.roles: [ master ]
3.Cat API _cat/master is replaced with _cat/cluster_manager
4.Deprecate master_timeout parameter to cluster_manager_timeout
5.Deprecate several interfaces, for example CatMasterMasterRecord
6.Replaces tests and comments

Issues Resolved

#221

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

kavilla

should be bwc

ananzh · 2022-04-21T20:56:42Z

should be bwc

will change to allow both master and cluster_manager

ananzh · 2022-04-26T20:28:26Z

Current test:

'use strict';
var host = 'localhost';
var protocol = 'http';
var port = 9200;
var auth = 'admin:admin'; // For testing only. Don't store credentials in code.
var ca_certs_path = '/full/path/to/root-ca.pem';
// Optional client certificates if you don't want to use HTTP basic authentication.
// var client_cert_path = '/full/path/to/client.pem'
// var client_key_path = '/full/path/to/client-key.pem'
// Create a client with SSL/TLS enabled.
var { Client } = require('@opensearch-project/opensearch');
var fs = require('fs');
var client = new Client({
      node: 'http://localhost:9200',
    //node: protocol + '://' + auth + '@' + host + ':' + port,
    //ssl: {
        //ca: fs.readFileSync(ca_certs_path),
        // You can turn off certificate verification (rejectUnauthorized: false) if you're using self-signed certificates with a hostname mismatch.
        // cert: fs.readFileSync(client_cert_path),
        // key: fs.readFileSync(client_key_path)
    //}
})
async function search() {
    // Create an index with non-default settings.
    var index_name = 'books'
    var settings = {
        'settings': {
            'index': {
                'number_of_shards': 4,
                'number_of_replicas': 3
            }
        }
    }
    var response = await client.indices.create({
        index: index_name,
        body: settings
    })
    console.log('Creating index:')
    console.log(response.body)
    
    var response = await client.cat.master();
    console.log('Cat master:')
    console.log(response.body)
    var response = await client.cat.cluster_manager();
    console.log('Cat cluster_manager:')
    console.log(response.body)
    // var time = '5s';
    // var response = await client.nodes.info({
    //     cluster_manager_timeout: time,
    // });
    // console.log('Nodes info:')
    // console.log(response.body)
    var time = '5s';
    var response = await client.cluster.health({
        master_timeout: time,
    });
    console.log('Cluster health:')
    console.log(response.body)
    var time = '5s';
    var response = await client.cluster.getSettings({
        master_timeout: time,
    });
    console.log('Cluster health:')
    console.log(response.body)
    
    // Add a document to the index.
    var document = {
        'title': 'The Outsider',
        'author': 'Stephen King',
        'year': '2018',
        'genre': 'Crime fiction'
    }
    var id = '1'
    var response = await client.index({
        id: id,
        index: index_name,
        body: document,
        refresh: true
    })
    console.log('Adding document:')
    console.log(response.body)
// Search for the document.
    var query = {
        'query': {
            'match': {
                'title': {
                    'query': 'The Outsider'
                }
            }
        }
    }
    var response = await client.search({
        index: index_name,
        body: query
    })
    console.log('Search results:')
    console.log(response.body.hits)
// Delete the document.
    var response = await client.delete({
        index: index_name,
        id: id
    })
    console.log('Deleting document:')
    console.log(response.body)
// Delete the index.
    var response = await client.indices.delete({
        index: index_name
    })
    console.log('Deleting index:')
    console.log(response.body)
}
search().catch(console.log)

ananzh · 2022-04-26T20:29:22Z

On cat.master

var response = await client.cat.master();
console.log('Cat master:')
console.log(response.body)

Result:

TypeError: client.cat.master is not a function
    at search (/home/anan/work/test/data.js:41:37)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Check this API without nodejs client. It is working.

curl localhost:9200/_cat/master
1jjPht0bRDqRuJWT_q6abQ 127.0.0.1 127.0.0.1 runTask-0

ananzh · 2022-04-26T20:30:56Z

On client.nodes.info

var time = '5s';
var response = await client.nodes.info({
      cluster_manager_timeout: time,
});
console.log('Nodes info:')
console.log(response.body)

Result:

{ ResponseError: illegal_argument_exception: [illegal_argument_exception] Reason: request [/_nodes] contains unrecognized parameter: [cluster_manager_timeout]
    at onBody (/home/anan/work/test/node_modules/@opensearch-project/opensearch/lib/Transport.js:374:23)
    at IncomingMessage.onEnd (/home/anan/work/test/node_modules/@opensearch-project/opensearch/lib/Transport.js:293:11)
    at IncomingMessage.emit (events.js:203:15)
    at endReadableNT (_stream_readable.js:1145:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
  name: 'ResponseError',
  meta:
   { body: { error: [Object], status: 400 },
     statusCode: 400,
     headers:
      { 'content-type': 'application/json; charset=UTF-8',
        'content-length': '289' },
     meta:
      { context: null,
        request: [Object],
        name: 'opensearch-js',
        connection: [Object],
        attempts: 0,
        aborted: false } } }

This is expected. If API exist but cluster_manager_timeout/master paras does not exist:
https://github.com/opensearch-project/OpenSearch/blob/c31cddf27e9618582b3cd175e4e0bce320af6a14/rest-api-spec/src/main/resources/rest-api-spec/api/nodes.info.json#L80

ananzh · 2022-04-26T20:33:46Z

On client.cluster.health
This works well.

var time = '5s';
    var response = await client.cluster.health({
        master_timeout: time,
    });
    console.log('Cluster health:')
    console.log(response.body)

Result:

Cluster health:
{ cluster_name: 'runTask',
  status: 'yellow',
  timed_out: false,
  number_of_nodes: 1,
  number_of_data_nodes: 1,
  discovered_master: true,
  discovered_cluster_manager: true,
  active_primary_shards: 4,
  active_shards: 4,
  relocating_shards: 0,
  initializing_shards: 0,
  unassigned_shards: 12,
  delayed_unassigned_shards: 0,
  number_of_pending_tasks: 0,
  number_of_in_flight_fetch: 0,
  task_max_waiting_in_queue_millis: 0,
  active_shards_percent_as_number: 25 }

api/opensearch_dashboards.d.ts

ananzh · 2022-04-26T21:02:44Z

Sorry my bad.

~/work/test$ node data.js
Creating index:
{ acknowledged: true, shards_acknowledged: true, index: 'books' }
Cat master:
1jjPht0bRDqRuJWT_q6abQ 127.0.0.1 127.0.0.1 runTask-0

Cluster health:
{ cluster_name: 'runTask',
  status: 'yellow',
  timed_out: false,
  number_of_nodes: 1,
  number_of_data_nodes: 1,
  discovered_master: true,
  discovered_cluster_manager: true,
  active_primary_shards: 4,
  active_shards: 4,
  relocating_shards: 0,
  initializing_shards: 0,
  unassigned_shards: 12,
  delayed_unassigned_shards: 0,
  number_of_pending_tasks: 0,
  number_of_in_flight_fetch: 0,
  task_max_waiting_in_queue_millis: 0,
  active_shards_percent_as_number: 25 }
Cluster health:
{ persistent: {}, transient: {} }
Adding document:
{ _index: 'books',
  _id: '1',
  _version: 1,
  result: 'created',
  forced_refresh: true,
  _shards: { total: 4, successful: 1, failed: 0 },
  _seq_no: 0,
  _primary_term: 1 }
Search results:
{ total: { value: 1, relation: 'eq' },
  max_score: 0.5753642,
  hits:
   [ { _index: 'books',
       _id: '1',
       _score: 0.5753642,
       _source: [Object] } ] }
Deleting document:
{ _index: 'books',
  _id: '1',
  _version: 2,
  result: 'deleted',
  _shards: { total: 4, successful: 1, failed: 0 },
  _seq_no: 1,
  _primary_term: 1 }
Deleting index:
{ acknowledged: true }

ananzh · 2022-04-26T21:36:10Z

cat.master and cat.cluster_manager are both supported.

var response = await client.cat.master();
console.log('Cat master:')
console.log(response.body)
var response = await client.cat.cluster_manager();
console.log('Cat cluster_manager:')
console.log(response.body)

Cat master:
Y6sYShIMRgiVfIUar5FVoA 127.0.0.1 127.0.0.1 runTask-0

Cat cluster_manager:
Y6sYShIMRgiVfIUar5FVoA 127.0.0.1 127.0.0.1 runTask-0

ananzh · 2022-04-26T21:41:01Z

Other test results:

anan@***:~/work/opensearch-js$ yarn test:unit
yarn run v1.22.5
$ tap test/unit/{*,**/*}.test.js
 SKIP  test/unit/esm/index.test.js
 ~ Skip because Node version < 12.17.0

 SKIP  test/unit/esm/index.test.js 1 skip of 1 3.378ms
 ~ Skip because Node version < 12.17.0

 PASS  test/unit/cloud-connection-pool.test.js 4 OK 14.881ms
 PASS  test/unit/selectors.test.js 8 OK 9.462ms
 PASS  test/unit/events.test.js 19 OK 33.137ms
 PASS  test/unit/errors.test.js 36 OK 29.054ms
 PASS  test/unit/base-connection-pool.test.js 44 OK 85.439ms
 PASS  test/unit/helpers/search.test.js 8 OK 24.873ms
 PASS  test/unit/serializer.test.js 19 OK 33.493ms
 PASS  test/unit/api.test.js 25 OK 73.844ms
 PASS  test/unit/connection-pool.test.js 90 OK 170.249ms
 PASS  test/unit/child.test.js 36 OK 152.741ms
test/unit/helpers/bulk.test.js 2> (node:3181474) DeprecationWarning: strictEqual() is deprecated, use equal() instead
test/unit/helpers/bulk.test.js 2> (node:3181474) DeprecationWarning: deepEqual() is deprecated, use same() instead
 PASS  test/unit/helpers/scroll.test.js 58 OK 188.939ms
 PASS  test/unit/helpers/bulk.test.js 291 OK 241.85ms
 PASS  test/unit/helpers/msearch.test.js 49 OK 568.246ms
 PASS  test/unit/connection.test.js 77 OK 3s
 PASS  test/unit/transport.test.js 202 OK 4s
 PASS  test/unit/client.test.js 118 OK 11s

                         
  🌈 SUMMARY RESULTS 🌈  
                         

 SKIP  test/unit/esm/index.test.js 1 skip of 1 3.378ms
 ~ Skip because Node version < 12.17.0

Suites:   17 passed, 17 of 17 completed
Asserts:  1084 passed, 1 skip, of 1085
Time:     12s
Done in 12.35s.
anan@***:~/work/opensearch-js$ yarn test:integration:helpers
yarn run v1.22.5
$ tap test/integration/helpers/*.test.js
 PASS  test/integration/helpers/search.test.js 11 OK 2s
 PASS  test/integration/helpers/msearch.test.js 27 OK 6s
 PASS  test/integration/helpers/scroll.test.js 215 OK 6s
 PASS  test/integration/helpers/bulk.test.js 25 OK 8s

                         
  🌈 SUMMARY RESULTS 🌈  
                         

Suites:   4 passed, 4 of 4 completed
Asserts:  278 passed, of 278
Time:     8s
Done in 8.82s.

ananzh · 2022-04-26T21:51:13Z

I would like to document some test steps:

Prepare
mkdir test cd test 
git clone https://github.com/opensearch-project/OpenSearch 
git clone https://github.com/opensearch-project/opensearch-js

//checkout OpenSearch 2.0
cd test/OpenSearch 
git checkout -b 2.0 origin/2.0
//Deploy local cluster (optional):
sudo sysctl -w vm.max_map_count=262144
ulimit -n 65535 
./deploy_single_tar_cluster.sh -v 1.0.0 -t releases -a x64 -s false
//run OpenSearch
./gradlew run -Drun.distribution=oss (run OpenSearch without security, this is fine for test purpose)
API Tests
//create another client folder in test
mkdir client
cd test/client and run npm init -y
// if opensearch-js package is installed, need to clean it first. Here are some clean steps:
rm -rf node-modules/@opensearch-project
clean yarn.lock (remove @opensearch-project part)
remove opensearch-project/opensearch in package.json
//Reinstall the local opensearch-js package. Do not checkot npm package
//Check package.json, make sure "types": "api/new.d.ts",
yarn add --dev '/home/anan/work/opensearch-js'
//double check package.json that opensearch-project/opensearch is installed and it is from your local
//create a test script
vim data.js and copy the above example
//run test script
node data.js
Unit Tests
yarn test:unit
Integration Tests
yarn test:integration:helpers

ananzh · 2022-04-26T23:40:07Z

[Research & Summary]

[Optional master_timeout]

—> What change:
For each master_timeout? , add anothercluster_manager_timeout?

—> Affected Files:

api/requestParams.d.ts
api/types.d.ts

—> Example:

export interface CatAllocation extends Generic {
  node_id?: string | string[];
  format?: string;
  bytes?: 'b' | 'k' | 'kb' | 'm' | 'mb' | 'g' | 'gb' | 't' | 'tb' | 'p' | 'pb';
  local?: boolean;
  cluster_manager_timeout?: string;
  master_timeout?: string;
  h?: string | string[];
  help?: boolean;
  s?: string | string[];
  v?: boolean;
}

—> How to Test:
There are two possible cases:

1.Only API exist but cluster_manager_timeout/master param does not exist.

For example, client.nodes.info:
https://github.com/opensearch-project/OpenSearch/blob/c31cddf27e9618582b3cd175e4e0bce320af6a14/rest-api-spec/src/main/resources/rest-api-spec/api/nodes.info.json#L80
We could see currently OpenSearch only support two params: timeout and flat_settings.

Use script to test it:

var response = await client.nodes.info({
         master_timeout: time,
     });
 
var response = await client.nodes.info({
         cluster_manager_timeout: time,
     });

Results are:

{ ResponseError: illegal_argument_exception: [illegal_argument_exception] Reason: request [/_nodes] contains unrecognized parameter: [master_timeout]
    at onBody (/home/anan/work/test/node_modules/@opensearch-project/opensearch/lib/Transport.js:374:23)
    at IncomingMessage.onEnd (/home/anan/work/test/node_modules/@opensearch-project/opensearch/lib/Transport.js:293:11)
    at IncomingMessage.emit (events.js:203:15)
    at endReadableNT (_stream_readable.js:1145:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
 
 
{ ResponseError: illegal_argument_exception: [illegal_argument_exception] Reason: request [/_nodes] contains unrecognized parameter: [cluster_manager_timeout]
    at onBody (/home/anan/work/test/node_modules/@opensearch-project/opensearch/lib/Transport.js:374:23)
    at IncomingMessage.onEnd (/home/anan/work/test/node_modules/@opensearch-project/opensearch/lib/Transport.js:293:11)
    at IncomingMessage.emit (events.js:203:15)
    at endReadableNT (_stream_readable.js:1145:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)

2.Both API and params exist
For example, client.cluster.health:
Master_timeout:
https://github.com/opensearch-project/OpenSearch/blob/70977525622053b662e79aa185034e21543d9a4e/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.health.json#L57
Cluster_manager_timeout:
https://github.com/opensearch-project/OpenSearch/blob/70977525622053b662e79aa185034e21543d9a4e/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.health.json#L65

Use script to test it:

var time = '5s';
var response = await client.cluster.getSettings({
      master_timeout: time,
});
console.log('Cluster getSettings use master_timeout:')
console.log(response.body)

var time = '5s';
var response = await client.cluster.getSettings({
      cluster_manager_timeout: time,
});
console.log('Cluster getSettings use cluster_manager_timeout:')
console.log(response.body)

Results are:

Cluster getSettings use master_timeout:
{ persistent: {}, transient: {} }
Cluster getSettings use cluster_manager_timeout:
{ persistent: {}, transient: {} }

[API's request query string]
—> What change:

Some APIs query params contains master_timeout. For example, cluster API:

const acceptedQuerystring = ['include_yes_decisions', 'include_disk_info', 'pretty', 'human', 'error_trace', 'source', 'filter_path', 'timeout', 'master_timeout', 'wait_for_removal', 'local', 'flat_settings', 'include_defaults', 'expand_wildcards', 'level', 'wait_for_active_shards', 'wait_for_nodes', 'wait_for_events', 'wait_for_no_relocating_shards', 'wait_for_no_initializing_shards', 'wait_for_status', 'node_ids', 'node_names', 'create', 'dry_run', 'explain', 'retry_failed', 'metric', 'wait_for_metadata_version', 'wait_for_timeout', 'ignore_unavailable', 'allow_no_indices']
const snakeCase = { includeYesDecisions: 'include_yes_decisions', includeDiskInfo: 'include_disk_info', errorTrace: 'error_trace', filterPath: 'filter_path', masterTimeout: 'master_timeout', waitForRemoval: 'wait_for_removal', flatSettings: 'flat_settings', includeDefaults: 'include_defaults', expandWildcards: 'expand_wildcards', waitForActiveShards: 'wait_for_active_shards', waitForNodes: 'wait_for_nodes', waitForEvents: 'wait_for_events', waitForNoRelocatingShards: 'wait_for_no_relocating_shards', waitForNoInitializingShards: 'wait_for_no_initializing_shards', waitForStatus: 'wait_for_status', nodeIds: 'node_ids', nodeNames: 'node_names', dryRun: 'dry_run', retryFailed: 'retry_failed', waitForMetadataVersion: 'wait_for_metadata_version', waitForTimeout: 'wait_for_timeout', ignoreUnavailable: 'ignore_unavailable', allowNoIndices: 'allow_no_indices' }

acceptedQuerystring is all the accepted query params for this API. For cluster API's method, query string's keys are transformed using snakeCase then pass to request.

 querystring = snakeCaseKeys(acceptedQuerystring, snakeCase, querystring)

For acceptedQuerystring, add 'cluster_manager_timeout'
For snakeCase, add clusterManagerTimeout: 'cluster_manager_timeout'

—> Affected Files:

api/cat.js
api/cluster.js
api/api/dangling_indices.js
api/api/delete_script.js
api/api/features.js
api/api/get_script.js
api/api/indices.js
api/api/ingest.js
api/api/put_script.js
api/api/snapshot.js

—> Example:

const acceptedQuerystring = ['format', 'local', 'h', 'help', 's', 'v', 'expand_wildcards', 'pretty', 'human', 'error_trace', 'source', 'filter_path', 'bytes', 'cluster_manager_timeout', 'master_timeout', 'fields', 'time', 'ts', 'health', 'pri', 'include_unloaded_segments', 'allow_no_match', 'allow_no_datafeeds', 'allow_no_jobs', 'from', 'size', 'full_id', 'include_bootstrap', 'active_only', 'detailed', 'index', 'ignore_unavailable', 'nodes', 'actions', 'parent_task_id']
const snakeCase = { expandWildcards: 'expand_wildcards', errorTrace: 'error_trace', filterPath: 'filter_path', clusterManagerTimeout: 'cluster_manager_timeout', masterTimeout: 'master_timeout', includeUnloadedSegments: 'include_unloaded_segments', allowNoMatch: 'allow_no_match', allowNoDatafeeds: 'allow_no_datafeeds', allowNoJobs: 'allow_no_jobs', fullId: 'full_id', includeBootstrap: 'include_bootstrap', activeOnly: 'active_only', ignoreUnavailable: 'ignore_unavailable', parentTaskId: 'parent_task_id' }

—> How to Test:
Similar to the previous case. Can use script to test.

[CAT.master API]
—> What change:
Add CatApi.prototype.cluster_manager
Add cluster_master method with corresponding renamed param and response types
(CatMaster --> CatClusterManager it is only used in Cat.master API)

—> Affected Files:

api/api/cat.js
index.d.ts
api/new.d.ts
api/opensearch_dashboards.d.ts

—> How to Test:
Similar to the previous case. Can use script to test.
cat.master and cat.cluster_manager are both supported.
Here is a script example:

var response = await client.cat.master();
console.log('Cat master:')
console.log(response.body)
var response = await client.cat.cluster_manager();
console.log('Cat cluster_manager:')
console.log(response.body)

Results:

Cat master:
Y6sYShIMRgiVfIUar5FVoA 127.0.0.1 127.0.0.1 runTask-0

Cat cluster_manager:
Y6sYShIMRgiVfIUar5FVoA 127.0.0.1 127.0.0.1 runTask-0

[NodeRole]
—> What change:

NodeRole/NodeRoles are defined in api/types.d.ts. They are used in:

1.NodeAttributes:
https://github.com/opensearch-project/opensearch-js/blob/13d6e0312dfe190cf9818b44cebbd580fb22f5f3/api/types.d.ts#L2053

NodeAttributes is called in ClusterRerouteRerouteState then called by ClusterRerouteResponse
NodeAttributes is called in ClusterStateResponse
NodeAttributes is called in SearchShardsResponse

2.NodesStats
https://github.com/opensearch-project/opensearch-js/blob/13d6e0312dfe190cf9818b44cebbd580fb22f5f3/api/requestParams.d.ts#L1264

NodeStats is called in NodesReloadSecureSettingsResponse
NodeStats is called in NodesStatsResponse

3.NodesInfoNodeInfo:
https://github.com/opensearch-project/opensearch-js/blob/13d6e0312dfe190cf9818b44cebbd580fb22f5f3/api/types.d.ts#L8780

NodesInfoNodeInfo is called in NodesInfoResponse

Add 'cluster_manager' in NodeRole

—> Affected Files:
api/types.d.ts

—> How to Test:
All response types. Should be no bwc issue.

[Connection role]
—> What change:
Add a new support role CLUSTER_MANAGER: 'cluster_manager'
Shrink the defaultRoles to only Connection.roles.DATA and Connection.roles.INGEST.

Connection.roles = { CLUSTER_MANAGER: 'cluster_manager', MASTER: 'master', DATA: 'data', INGEST: 'ingest'}

const defaultRoles = {
  [Connection.roles.DATA]: true,
  [Connection.roles.INGEST]: true
}

—> Affected Files:
lib/Connection.js

—> How to Test:
Can be tested in unit tests.

[Optional initial_master_nodes]
—> What change:
Add optional initial_cluster_manager_nodes?: string

—> Affected Files:
api/types.d.ts

—> Example:

export interface NodesInfoNodeInfoSettingsCluster {
  name: Name
  routing?: IndicesIndexRouting
  election: NodesInfoNodeInfoSettingsClusterElection
  initial_cluster_manager_nodes?: string
  initial_master_nodes?: string
}

—> How to Test:
NodesInfoNodeInfoSettingsCluster is only used in NodesInfoNodeInfoSettings. Then called by interface NodesInfoNodeInfo and then called by interface NodesInfoResponse.
Only affect response. Should be no bwc issue.

[Optional master_node param]
—> What change:
Optional master_node is used in two places, ClusterRerouteRerouteState and ClusterStateResponse.
Add optional cluster_manager_node. See below example section.

—> Affected Files:
api/types.d.ts

—> Example:

export interface ClusterStateResponse {
  cluster_name: Name
  cluster_uuid: Uuid
  cluster_manager_node?: string
  master_node?: string
  state?: string[]
  state_uuid?: Uuid
  version?: VersionNumber
  blocks?: ClusterStateClusterStateBlocks
  metadata?: ClusterClusterStateMetadata
  nodes?: Record<NodeName, NodeAttributes>
  routing_table?: Record<string, EmptyObject>
  routing_nodes?: ClusterClusterStateRoutingNodes
  snapshots?: ClusterClusterStateSnapshots
  snapshot_deletions?: ClusterClusterStateDeletedSnapshots
}

—> How to Test:
ClusterRerouteRerouteState is called in ClusterRerouteResponse which is a response.
ClusterStateResponse is a response.
Only affect response. Should be no bwc issue.

[Optional master param]
—> What change:
There are two interfaces CatNodesNodesRecord and ClusterStatsClusterNodeCount defined in types will be affected.
Add optional cluster_manager.

—> Affected Files:
api/types.d.ts

—> Example:

export interface ClusterStatsClusterNodeCount {
  coordinating_only: integer
  data: integer
  ingest: integer
  cluster_manager?: integer
  master?: integer
  total: integer
  voting_only: integer
  remote_cluster_client: integer
}

—> How to Test:
CatNodesNodesRecord is only called in CatNodesResponse which is a response.
ClusterStatsClusterNodeCount only affects ClusterStatsResponse.
Only affect response. Should be no bwc issue.

ashwin-pc · 2022-04-27T00:19:40Z

@ananzh there ares some lint error's, can you fix them?

joshuarrrr

I had a few questions about the way the new roles were implemented, but those may be due to my lack of familiarity.

Where possible, I'd recommend just aliasing the old terminology and names to the new names, to avoid drift or divergence in the duplicated code paths.

api/api/cat.js

api/types.d.ts

test/types/connection.test-d.ts

lib/Connection.js

joshuarrrr · 2022-04-27T18:02:30Z

test/unit/base-connection-pool.test.js

-          roles: ['master', 'data', 'ingest']
+          roles: ['cluster_manager', 'data', 'ingest']


Just a note - changing all the test roles in this file is actually removing test coverage for the master role. Ideally, we should still test the deprecated role until we actually remove support for it.

Yeah, so the purpose of changing some tests is to make sure both cluster_manager and master are working. Instead of writing very similar unit tests for these two roles, I decide to modify some current unit tests, especially if there are multiple nodes. Would like more discussion here.

I think confirming that both options work the same is worthwhile in any of the tests that currently use master.

ananzh · 2022-04-28T04:13:27Z

@ananzh there ares some lint error's, can you fix them?

done

Updates made

dblock · 2022-05-03T15:47:22Z

What's the upgrade path for users that have applications that work against OpenSearch 1.x and want to go to 2.x?

Ideally 2.0 client would work against 1.x, see opensearch-project/opensearch-clients#17

kavilla · 2022-05-05T22:30:49Z

What's the upgrade path for users that have applications that work against OpenSearch 1.x and want to go to 2.x?

Ideally 2.0 client would work against 1.x, see opensearch-project/opensearch-clients#17

I believe mentioned in the thread, assuming none of the new APIs are used it should ideally we work. But I know there was another issue about the source of truth for compatibility. It seems fairly easy to remember clients major versions only equal to the same major version of OpenSearch. I do see the RFC for API versioning which will also effect decisions here. I think for sure it should be standardized across all clients.

kavilla

Some comments, but pulled it down in my OpenSearch Dashboards and didn't run into any issues (albeit didn't change any configurations).

Thanks for this!

lib/Transport.js

test/unit/client.test.js

api/requestParams.d.ts

index.d.ts

lib/pool/BaseConnectionPool.js

kavilla · 2022-05-06T06:56:30Z

Also seems like this file used for generating the API file https://github.com/opensearch-project/opensearch-js/blob/main/scripts/utils/patch.json and marking as deprecated.

tmarkley

I think that instead of choosing some tests to replace master, each of the test suites should confirm that both master and the alternative behave the same way.

api/types.d.ts

test/acceptance/sniff.test.js

test/types/connection.test-d.ts

tmarkley · 2022-05-18T15:01:08Z

test/unit/base-connection-pool.test.js

-          roles: ['master', 'data', 'ingest']
+          roles: ['cluster_manager', 'data', 'ingest']


I think confirming that both options work the same is worthwhile in any of the tests that currently use master.

1.Deprecate setting cluster.initial_master_nodes and introduce the alternative setting cluster.initial_cluster_manager_nodes. 2.Use a new node role cluster_manager that has the same functionality with master in the node setting node.roles: [ master ] 3.Cat API _cat/master is deprecated with _cat/cluster_manager 4.Deprecate master_timeout parameter to cluster_manager_timeout 5.Deprecate several interfaces, for example CatMasterMasterRecord 6.Replaces tests and comments Issue Resolved: opensearch-project#221 Signed-off-by: Anan Zhuang <[email protected]> add some comments and toDos for master deprecation Signed-off-by: Anan Zhuang <[email protected]> update TODO and use JSDoc Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>

tmarkley · 2022-05-25T18:48:26Z

lib/Transport.js

+  if (
+    (node.roles.cluster_manager === true || node.roles.master === true) &&
+    node.roles.data === false &&
+    node.roles.ingest === false
+  ) {


Do we have unit tests for this function?

ananzh requested review from kavilla, boktorbb and tmarkley April 21, 2022 07:04

ananzh self-assigned this Apr 21, 2022

ananzh added 📛 rename 2.0 labels Apr 21, 2022

kavilla previously requested changes Apr 21, 2022

View reviewed changes

kavilla reviewed Apr 26, 2022

View reviewed changes

api/opensearch_dashboards.d.ts Outdated Show resolved Hide resolved

ananzh force-pushed the master-rename branch from 8fac57a to 8b6ca71 Compare April 26, 2022 20:56

ananzh force-pushed the master-rename branch from 8b6ca71 to 21dad80 Compare April 27, 2022 00:47

joshuarrrr reviewed Apr 27, 2022

View reviewed changes

ananzh force-pushed the master-rename branch from 21dad80 to 238ce88 Compare May 4, 2022 22:31

ananzh requested a review from a team as a code owner May 4, 2022 22:31

kavilla reviewed May 6, 2022

View reviewed changes

lib/Transport.js Outdated Show resolved Hide resolved

test/unit/client.test.js Outdated Show resolved Hide resolved

api/requestParams.d.ts Show resolved Hide resolved

index.d.ts Show resolved Hide resolved

lib/pool/BaseConnectionPool.js Outdated Show resolved Hide resolved

ananzh force-pushed the master-rename branch 2 times, most recently from 9c03f6f to d1478ec Compare May 12, 2022 02:32

tmarkley mentioned this pull request May 16, 2022

Deprecate the "Master" nomenclature opensearch-project/OpenSearch-Dashboards#1318

Closed

8 tasks

kavilla approved these changes May 18, 2022

View reviewed changes

tmarkley suggested changes May 18, 2022

View reviewed changes

ananzh force-pushed the master-rename branch 2 times, most recently from 0ae26de to 95ee7a9 Compare May 24, 2022 23:27

add more unit test to test cluster_manager role

3ec9f0d

Signed-off-by: Anan Zhuang <[email protected]>

ananzh force-pushed the master-rename branch from 95ee7a9 to 3ec9f0d Compare May 24, 2022 23:35

tmarkley approved these changes May 25, 2022

View reviewed changes

ananzh merged commit e008fe8 into opensearch-project:main May 25, 2022

kavilla linked an issue May 26, 2022 that may be closed by this pull request

Deprecate the "Master" nomenclature in nodejs client #221

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2.0] Deprecate the "Master" nomenclature in nodejs client #222

[2.0] Deprecate the "Master" nomenclature in nodejs client #222

ananzh commented Apr 21, 2022

kavilla left a comment

ananzh commented Apr 21, 2022

ananzh commented Apr 26, 2022 •

edited

Loading

ananzh commented Apr 26, 2022 •

edited

Loading

ananzh commented Apr 26, 2022 •

edited

Loading

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ashwin-pc commented Apr 27, 2022

joshuarrrr left a comment

joshuarrrr Apr 27, 2022

ananzh Apr 28, 2022

tmarkley May 18, 2022

ananzh commented Apr 28, 2022

dblock commented May 3, 2022 •

edited

Loading

kavilla commented May 5, 2022

kavilla left a comment

kavilla commented May 6, 2022

tmarkley left a comment

tmarkley May 18, 2022

tmarkley May 25, 2022

		roles: ['master', 'data', 'ingest']
		roles: ['cluster_manager', 'data', 'ingest']

[2.0] Deprecate the "Master" nomenclature in nodejs client #222

[2.0] Deprecate the "Master" nomenclature in nodejs client #222

Conversation

ananzh commented Apr 21, 2022

Description

Issues Resolved

Check List

kavilla left a comment

Choose a reason for hiding this comment

ananzh commented Apr 21, 2022

ananzh commented Apr 26, 2022 • edited Loading

ananzh commented Apr 26, 2022 • edited Loading

ananzh commented Apr 26, 2022 • edited Loading

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ananzh commented Apr 26, 2022

ashwin-pc commented Apr 27, 2022

joshuarrrr left a comment

Choose a reason for hiding this comment

joshuarrrr Apr 27, 2022

Choose a reason for hiding this comment

ananzh Apr 28, 2022

Choose a reason for hiding this comment

tmarkley May 18, 2022

Choose a reason for hiding this comment

ananzh commented Apr 28, 2022

dblock commented May 3, 2022 • edited Loading

kavilla commented May 5, 2022

kavilla left a comment

Choose a reason for hiding this comment

kavilla commented May 6, 2022

tmarkley left a comment

Choose a reason for hiding this comment

tmarkley May 18, 2022

Choose a reason for hiding this comment

tmarkley May 25, 2022

Choose a reason for hiding this comment

ananzh commented Apr 26, 2022 •

edited

Loading

ananzh commented Apr 26, 2022 •

edited

Loading

ananzh commented Apr 26, 2022 •

edited

Loading

dblock commented May 3, 2022 •

edited

Loading