You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/04/28 10:35:49 UTC
[GitHub] [ignite] ptupitsyn opened a new pull request #9058: IGNITE-14655 .NET: DataStreamer async API improvements
ptupitsyn opened a new pull request #9058:
URL: https://github.com/apache/ignite/pull/9058
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ptupitsyn commented on a change in pull request #9058: IGNITE-14655 .NET: Improve DataStreamer API
Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #9058:
URL: https://github.com/apache/ignite/pull/9058#discussion_r622965432
##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Datastream/IDataStreamer.cs
##########
@@ -168,55 +184,90 @@ public interface IDataStreamer<TK, TV> : IDisposable
IStreamReceiver<TK, TV> Receiver { get; set; }
/// <summary>
- /// Adds single key-value pair for loading. Passing <c>null</c> as value will be
+ /// Adds single key-value pair for loading. Passing <c>null</c> as value will be
/// interpreted as removal.
/// </summary>
/// <param name="key">Key.</param>
/// <param name="val">Value.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Add.")]
Task AddData(TK key, TV val);
/// <summary>
- /// Adds single key-value pair for loading. Passing <c>null</c> as pair's value will
+ /// Adds single key-value pair for loading. Passing <c>null</c> as pair's value will
/// be interpreted as removal.
/// </summary>
/// <param name="pair">Key-value pair.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Add.")]
Task AddData(KeyValuePair<TK, TV> pair);
/// <summary>
- /// Adds collection of key-value pairs for loading.
+ /// Adds collection of key-value pairs for loading.
/// </summary>
/// <param name="entries">Entries.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Add.")]
Task AddData(ICollection<KeyValuePair<TK, TV>> entries);
/// <summary>
/// Adds key for removal.
/// </summary>
/// <param name="key">Key.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Remove.")]
Task RemoveData(TK key);
/// <summary>
- /// Makes an attempt to load remaining data. This method is mostly similar to
- /// <see cref="IDataStreamer{K,V}.Flush()"/> with the difference that it won't wait and
+ /// Adds single key-value pair for loading. Passing <c>null</c> as value will be
+ /// interpreted as removal.
+ /// </summary>
+ /// <param name="key">Key.</param>
+ /// <param name="val">Value.</param>
+ void Add(TK key, TV val);
+
+ /// <summary>
+ /// Adds single key-value pair for loading. Passing <c>null</c> as pair's value will
+ /// be interpreted as removal.
+ /// </summary>
+ /// <param name="pair">Key-value pair.</param>
+ void Add(KeyValuePair<TK, TV> pair);
+
+ /// <summary>
+ /// Adds collection of key-value pairs for loading.
+ /// </summary>
+ /// <param name="entries">Entries.</param>
+ void Add(ICollection<KeyValuePair<TK, TV>> entries);
+
+ /// <summary>
+ /// Adds key for removal.
+ /// </summary>
+ /// <param name="key">Key.</param>
+ void Remove(TK key);
+
+ /// <summary>
+ /// Makes an attempt to load remaining data. This method is mostly similar to
+ /// <see cref="IDataStreamer{K,V}.Flush()"/> with the difference that it won't wait and
/// will exit immediately.
/// </summary>
+ [Obsolete("Use FlushAsync")]
void TryFlush();
/// <summary>
- /// Loads any remaining data, but doesn't close the streamer. Data can be still added after
- /// flush is finished. This method blocks and doesn't allow to add any data until all data
- /// is loaded.
+ /// Loads any remaining buffered data, but doesn't close the streamer.
/// </summary>
void Flush();
+ /// <summary>
+ /// Loads any remaining buffered data, but doesn't close the streamer.
+ /// </summary>
+ Task FlushAsync();
+
/// <summary>
/// Closes this streamer optionally loading any remaining data.
/// </summary>
/// <param name="cancel">Whether to cancel ongoing loading operations. When set to <c>true</c>
- /// there is not guarantees what data will be actually loaded to cache.</param>
+ /// there are no guarantees what data will be actually loaded to cache.</param>
Review comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ptupitsyn merged pull request #9058: IGNITE-14655 .NET: Improve DataStreamer API
Posted by GitBox <gi...@apache.org>.
ptupitsyn merged pull request #9058:
URL: https://github.com/apache/ignite/pull/9058
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] isapego commented on a change in pull request #9058: IGNITE-14655 .NET: Improve DataStreamer API
Posted by GitBox <gi...@apache.org>.
isapego commented on a change in pull request #9058:
URL: https://github.com/apache/ignite/pull/9058#discussion_r622512433
##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Datastream/IDataStreamer.cs
##########
@@ -138,24 +134,44 @@ public interface IDataStreamer<TK, TV> : IDisposable
/// <para />
/// Setter must be called before any add/remove operation.
/// <para />
- /// Default is 0, which means Ignite calculates this automatically as
- /// <see cref="IgniteConfiguration.DataStreamerThreadPoolSize"/> *
+ /// Default is 0, which means Ignite calculates this automatically as
+ /// <see cref="IgniteConfiguration.DataStreamerThreadPoolSize"/> *
/// <see cref="DataStreamerDefaults.DefaultParallelOperationsMultiplier"/>.
/// </summary>
int PerNodeParallelOperations { get; set; }
/// <summary>
- /// Automatic flush frequency in milliseconds. Essentially, this is the time after which the
- /// streamer will make an attempt to submit all data added so far to remote nodes.
- /// Note that there is no guarantee that data will be delivered after this concrete
- /// attempt (e.g., it can fail when topology is changing), but it won't be lost anyway.
+ /// Gets or sets the automatic flush frequency, in milliseconds.
+ /// Data streamer buffers the data for performance reasons.
+ /// The buffer is flushed in the following cases:
+ /// <ul>
+ /// <li>Buffer is full.</li>
+ /// <li><see cref="Flush"/> or <see cref="TryFlush"/> is called.</li>
+ /// <li>Periodically when <see cref="AutoFlushInterval"/> is set.</li >
+ /// </ul>
/// <para />
/// If set to <c>0</c>, automatic flush is disabled.
/// <para />
/// Default is <c>0</c> (disabled).
/// </summary>
+ [Obsolete("Use AutoFlushInterval.")]
Review comment:
Should not there be a justification for deprecation?
##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Datastream/IDataStreamer.cs
##########
@@ -168,55 +184,90 @@ public interface IDataStreamer<TK, TV> : IDisposable
IStreamReceiver<TK, TV> Receiver { get; set; }
/// <summary>
- /// Adds single key-value pair for loading. Passing <c>null</c> as value will be
+ /// Adds single key-value pair for loading. Passing <c>null</c> as value will be
/// interpreted as removal.
/// </summary>
/// <param name="key">Key.</param>
/// <param name="val">Value.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Add.")]
Task AddData(TK key, TV val);
/// <summary>
- /// Adds single key-value pair for loading. Passing <c>null</c> as pair's value will
+ /// Adds single key-value pair for loading. Passing <c>null</c> as pair's value will
/// be interpreted as removal.
/// </summary>
/// <param name="pair">Key-value pair.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Add.")]
Task AddData(KeyValuePair<TK, TV> pair);
/// <summary>
- /// Adds collection of key-value pairs for loading.
+ /// Adds collection of key-value pairs for loading.
/// </summary>
/// <param name="entries">Entries.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Add.")]
Task AddData(ICollection<KeyValuePair<TK, TV>> entries);
/// <summary>
/// Adds key for removal.
/// </summary>
/// <param name="key">Key.</param>
/// <returns>Task for this operation.</returns>
+ [Obsolete("Use Remove.")]
Task RemoveData(TK key);
/// <summary>
- /// Makes an attempt to load remaining data. This method is mostly similar to
- /// <see cref="IDataStreamer{K,V}.Flush()"/> with the difference that it won't wait and
+ /// Adds single key-value pair for loading. Passing <c>null</c> as value will be
+ /// interpreted as removal.
+ /// </summary>
+ /// <param name="key">Key.</param>
+ /// <param name="val">Value.</param>
+ void Add(TK key, TV val);
+
+ /// <summary>
+ /// Adds single key-value pair for loading. Passing <c>null</c> as pair's value will
+ /// be interpreted as removal.
+ /// </summary>
+ /// <param name="pair">Key-value pair.</param>
+ void Add(KeyValuePair<TK, TV> pair);
+
+ /// <summary>
+ /// Adds collection of key-value pairs for loading.
+ /// </summary>
+ /// <param name="entries">Entries.</param>
+ void Add(ICollection<KeyValuePair<TK, TV>> entries);
+
+ /// <summary>
+ /// Adds key for removal.
+ /// </summary>
+ /// <param name="key">Key.</param>
+ void Remove(TK key);
+
+ /// <summary>
+ /// Makes an attempt to load remaining data. This method is mostly similar to
+ /// <see cref="IDataStreamer{K,V}.Flush()"/> with the difference that it won't wait and
/// will exit immediately.
/// </summary>
+ [Obsolete("Use FlushAsync")]
void TryFlush();
/// <summary>
- /// Loads any remaining data, but doesn't close the streamer. Data can be still added after
- /// flush is finished. This method blocks and doesn't allow to add any data until all data
- /// is loaded.
+ /// Loads any remaining buffered data, but doesn't close the streamer.
/// </summary>
void Flush();
+ /// <summary>
+ /// Loads any remaining buffered data, but doesn't close the streamer.
+ /// </summary>
+ Task FlushAsync();
+
/// <summary>
/// Closes this streamer optionally loading any remaining data.
/// </summary>
/// <param name="cancel">Whether to cancel ongoing loading operations. When set to <c>true</c>
- /// there is not guarantees what data will be actually loaded to cache.</param>
+ /// there are no guarantees what data will be actually loaded to cache.</param>
Review comment:
Actually, "there is no guarantee" sounds more authentic to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ptupitsyn commented on a change in pull request #9058: IGNITE-14655 .NET: Improve DataStreamer API
Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #9058:
URL: https://github.com/apache/ignite/pull/9058#discussion_r622968619
##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Datastream/IDataStreamer.cs
##########
@@ -138,24 +134,44 @@ public interface IDataStreamer<TK, TV> : IDisposable
/// <para />
/// Setter must be called before any add/remove operation.
/// <para />
- /// Default is 0, which means Ignite calculates this automatically as
- /// <see cref="IgniteConfiguration.DataStreamerThreadPoolSize"/> *
+ /// Default is 0, which means Ignite calculates this automatically as
+ /// <see cref="IgniteConfiguration.DataStreamerThreadPoolSize"/> *
/// <see cref="DataStreamerDefaults.DefaultParallelOperationsMultiplier"/>.
/// </summary>
int PerNodeParallelOperations { get; set; }
/// <summary>
- /// Automatic flush frequency in milliseconds. Essentially, this is the time after which the
- /// streamer will make an attempt to submit all data added so far to remote nodes.
- /// Note that there is no guarantee that data will be delivered after this concrete
- /// attempt (e.g., it can fail when topology is changing), but it won't be lost anyway.
+ /// Gets or sets the automatic flush frequency, in milliseconds.
+ /// Data streamer buffers the data for performance reasons.
+ /// The buffer is flushed in the following cases:
+ /// <ul>
+ /// <li>Buffer is full.</li>
+ /// <li><see cref="Flush"/> or <see cref="TryFlush"/> is called.</li>
+ /// <li>Periodically when <see cref="AutoFlushInterval"/> is set.</li >
+ /// </ul>
/// <para />
/// If set to <c>0</c>, automatic flush is disabled.
/// <para />
/// Default is <c>0</c> (disabled).
/// </summary>
+ [Obsolete("Use AutoFlushInterval.")]
Review comment:
The justification is that `TimeSpan` should be used for time interval purposes
* No ambiguity about units
* .NET-native type
* Consistent with other Ignite APIs and system APIs
However, I don't think we should provide those details in the deprecation message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org