TS-4265: Thread initialization update. #3

SolidWallOfCode · 2017-03-12T15:39:29Z

No description provided.

PSUdaemon · 2017-03-14T19:25:59Z

iocore/cluster/P_ClusterCacheInternal.h

-    }
-    return false;
+    auto &tg = eventProcessor.thread_group[ET_CLUSTER];
+    return ET_CLUSTER && std::any_of(tg._thread, tg._thread + tg._count, [et](EThread *t) { return t == et; });


When is it valid that a thread in the ET_CLUSTER thread_group not be of type ET_CLUSTER?

That's how the current code works. The type is a bit mask. But more realistically, it's cluster code, it should never be called. But I might want to put the ET_CLUSTER check first - if it's zero then the cluster group was never set up.

PSUdaemon · 2017-03-14T19:30:08Z

iocore/cache/I_Cache.h

  }

-  virtual int start(int n_cache_threads = 0, size_t stacksize = DEFAULT_STACKSIZE);
+  virtual int start(int n_cache_threads = 0, size_t stacksize = DEFAULT_STACK_SIZE) override;


How come the name of the constant is changed to add an underscore but the variable name is not?

Heh. I'll look at that.

PSUdaemon · 2017-03-14T19:36:14Z

iocore/eventsystem/I_EventProcessor.h

+
+  /// Convenience overload.
+  /// This registers @a name as an event type using @c registerEventType and then calls the real @c spawn_event_threads
+  EventType spawn_event_threads(const char *name, int n_thread, size_t stacksize = DEFAULT_STACK_SIZE);


This feels icky to me. 🚲

It's handy in several places.

PSUdaemon · 2017-03-14T20:38:20Z

doc/developer-guide/architecture/threads-and-events.en.rst

+   eventProcessor.spawn_event_threads(ET_GROUP, n_group_threads, GROUP_STACK_SIZE);
+
+
+The function :code:`Group_Thread_Init` can be replaced with a continuation if that's more convenient. One advantage of a continuation is additional data (via :arg:`cookie`) can be provide during thread initialization.


Is edata being replaced with cookie? Seems like a name collision to use cookie.

I was mixed about that. It's called a cookie in the Event structure, which is where the argument name comes from.

PSUdaemon · 2017-03-14T21:24:17Z

iocore/eventsystem/UnixEThread.cc

-    signal_hook(nullptr),
-    tt(att),
-    server_session_pool(nullptr)
+EThread::EThread(ThreadType att, int anid) : id(anid), tt(att), server_session_pool(NULL)


Why changing nullptr to NULL?

Because this was done initially before the Great Null Shift and I missed this during the merge.

PSUdaemon · 2017-03-14T21:38:35Z

iocore/eventsystem/UnixEventProcessor.cc

+  /// Set the affinity for the current thread.
+  int set_affinity(int, Event *);
+
+#if defined(HAVE_HWLOC_OBJ_PU)


Shouldn't this be #if defined(TS_USE_HWLOC)?

PSUdaemon · 2017-03-14T21:41:05Z

iocore/eventsystem/UnixEventProcessor.cc

+    Debug("iocore_thread", "EThread: %d %s: %d CPU Mask: %s\n", i, _name, obj->logical_index, cpu_mask);
 #else
-      Debug("iocore_thread", "EThread: %d %s: %d", i, obj_name, obj->logical_index);
+    Debug("iocore_thread", "EThread: %d %s: %d\n", i, _name, obj->logical_index);


Why adding newline char here?

Standardize error messages by removing the terminating period.

… statement

Coverity 1199997 Coverity 1200000 Coverity 1200003 Coverity 1200012

This also undoes a previous change, that hopefully the new model rule will take care of (instead of nesting the if/else).

The name of this type is misspelled in the documentation. Signed-off-by: David Calavera <david.calavera@gmail.com>

This is for the GNU indent tool, which has been superseded by clang-format.

Let's use some C++11 features!

…ter. TS_LUA_CONFIG_HTTP_NEGATIVE_CACHING_LIFETIME was mapped to the wrong TS_CONFIG_... value. This patch corrects the mapping thus allowing the negative caching lifetime to be changed via the Lua plugin.

The file is empty so I don't think we need it.

Replaced all calls to do_io(VIO::ENUM, ...) with matching do_io_*(...).

The eventProcessor.spawn_thread() is used to create a DEDICATED EThread. The n_dthreads is total number of DEDICATED EThreads. When a new DEDICATED EThread created, it makes the `all_dthreads[n_dthreads]' point to the EThread object then do `n_dthreads++' It is not likes the spawn_event_threads() that only called from main() before any EThread runs. It is can be called from main() and/or any EThread. Therefore there is a race on accessing `all_dthreads[n_dthreads]' and `n_dthreads++'.

…mitted

…or a valid request.

# This is the 1st commit message: BufferWriter: Formatting tweaks, updated documentation. # The commit message #2 will be skipped: # BufferWriterFormat: Add support for sockaddr and IpEndpoint. # The commit message #3 will be skipped: # BufferWriter: Cleanup pass. # Exchange align '^' and '=' to conform to documentation and Python spec.

Fix the `make check` failure with the following output by changing to .gold which meets our license exclusion regex rules: Printing headers for text files without a valid license header... ===================================================== == File: ./tests/gold_tests/records/gold/renamed_records.out ===================================================== ``` ┌■ 8 Renamed records: └┬──» #1 : proxy.config.output.logfile -> proxy.config.output.logfile.name ├──» #2 : proxy.config.exec_thread.autoconfig -> proxy.config.exec_thread.autoconfig.enabled ├──» #3 : proxy.config.hostdb -> proxy.config.hostdb.enabled ├──» #4 : proxy.config.tunnel.prewarm -> proxy.config.tunnel.prewarm.enabled ├──» #5 : proxy.config.ssl.origin_session_cache -> proxy.config.ssl.origin_session_cache.enabled ├──» #6 : proxy.config.ssl.session_cache -> proxy.config.ssl.session_cache.value ├──» #7 : proxy.config.ssl.TLSv1_3 -> proxy.config.ssl.TLSv1_3.enabled └──» #8 : proxy.config.ssl.client.TLSv1_3 -> proxy.config.ssl.client.TLSv1_3.enabled ```

PSUdaemon reviewed Mar 14, 2017

View reviewed changes

PSUdaemon and others added 23 commits May 15, 2017 17:13

Coverity CID #1241990 Resource leak

5dc703a

Remove ending period from error messages.

edc425d

Standardize error messages by removing the terminating period.

remove the configuration to enable or disable safe requests retry

cacdb55

Coverity_1022180_1022181: ka_action can't be KA_UNKNOWN in the switch…

0a84b10

… statement

Coverity CID #1241990 Resource leak

56ed25f

proxy.config.diags.debug.enabled is reloadable

c1303d4

Various Coverity fixes for logstats

430993b

Coverity 1199997 Coverity 1200000 Coverity 1200003 Coverity 1200012

Coverity CID #1022089 Copy into fixed size buffer

32c4975

Coverity CID #1022090 Copy into fixed size buffer

0931547

Coverity CID #1367529 Copy into fixed size buffer

ba70668

Adds a coverity model file, and excludes a stat() check

f29b920

This also undoes a previous change, that hopefully the new model rule will take care of (instead of nesting the if/else).

Coverity_1021917: closet_match can not be null at that point

c91c390

Coverity CID #1022090 Copy into fixed size buffer

861ccbd

Add a couple of CI build scripts, to source from Jenkins

ade891f

coredump with server_session

1f8d00d

update epel version.

cf93f6e

remap test for secure communication with origin server

0306c44

Fix TSMilestonesType documentation.

d67812d

The name of this type is misspelled in the documentation. Signed-off-by: David Calavera <david.calavera@gmail.com>

Delete .indent.pro

a578f0d

This is for the GNU indent tool, which has been superseded by clang-format.

Issue_1943: get rid of ticket_key_name tag from ssl_multicert

ec7be29

Clean up BaseLogFile

b817429

Let's use some C++11 features!

Plugin ts_lua - Incorrect mapping of transaction configuration parame…

48a906a

…ter. TS_LUA_CONFIG_HTTP_NEGATIVE_CACHING_LIFETIME was mapped to the wrong TS_CONFIG_... value. This patch corrects the mapping thus allowing the negative caching lifetime to be changed via the Lua plugin.

obj_found is never be true

28a6101

scw00 and others added 20 commits May 23, 2017 15:38

coredump_handleEvent fix

7a71977

Delete .gitmodules

87338da

The file is empty so I don't think we need it.

Bump config delete timeout to 5min

2ab6c6c

ignore .vscode/

17e5e5c

Deprecated VConnection do_io

b594786

Replaced all calls to do_io(VIO::ENUM, ...) with matching do_io_*(...).

Removes the tsqa tests, to avoid confusion. Use autest.

1e41700

Fixed problem with url_len sometimes not being initialized

75a9f61

Use proper make wrapper, and parallelize

d8582f0

More cleanup of the CI scripts

54b7bac

Coverity 1021693: Uninitialized pointer field

6c2081e

Log the Lua error message when evaluating metrics fails.

8e85244

Doc: Fixup milestones documentation.

fa8b73c

Doc: Fix up some TS SSL API documentation warnings.

80164cd

Updated clang-tidy default config to be the option that have been com…

15c34d6

…mitted

coverity 1224618: Uninitialized scalar field

560148b

coverity 1196479: Uninitialized pointer field

1c0962e

Output the responding url when "Warning: Document truncated" occurs f…

2fc651d

…or a valid request.

TS-4265: Thread initialization update.

2bf169e

TS-4265: Ready for testing.

fe9e646

SolidWallOfCode force-pushed the ts-4265 branch from 46bad24 to fe9e646 Compare May 28, 2017 17:24

SolidWallOfCode added 2 commits May 28, 2017 21:59

TS-4265: Fix start up race condition.

ed3fd87

TS-4265: The Phil Edit.

fc21288

SolidWallOfCode closed this May 29, 2017

		eventProcessor.spawn_event_threads(ET_GROUP, n_group_threads, GROUP_STACK_SIZE);


		The function :code:`Group_Thread_Init` can be replaced with a continuation if that's more convenient. One advantage of a continuation is additional data (via :arg:`cookie`) can be provide during thread initialization.

TS-4265: Thread initialization update. #3

TS-4265: Thread initialization update. #3

Uh oh!

Conversation

SolidWallOfCode commented Mar 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

17 participants